From 83bd2eb5e0be11b400066866b96fe8743209a63d Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Mon, 6 Nov 2023 13:54:06 +0100 Subject: [PATCH 01/16] PB-28551 Fix regularly failing tests --- .../tests/Factory/EmailQueueFactory.php | 2 +- .../UsersEditAdminRoleRevokedControllerTest.php | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php b/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php index ec606b48d4..9e39c1eb9e 100644 --- a/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php +++ b/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php @@ -48,7 +48,7 @@ protected function setDefaultTemplate(): void $this->setDefaultData(function (Generator $faker) { $email = $faker->email(); $title = $faker->sentence(); - $locale = $faker->locale(); + $locale = 'en-UK'; return [ 'email' => $email, diff --git a/tests/TestCase/Controller/Users/UsersEditAdminRoleRevokedControllerTest.php b/tests/TestCase/Controller/Users/UsersEditAdminRoleRevokedControllerTest.php index d96f2359a9..64f7b7d8f7 100644 --- a/tests/TestCase/Controller/Users/UsersEditAdminRoleRevokedControllerTest.php +++ b/tests/TestCase/Controller/Users/UsersEditAdminRoleRevokedControllerTest.php @@ -48,7 +48,10 @@ public function setUp(): void public function testUsersEditAdminRoleRevokedController_NotificationEnabled(): void { - $jane = UserFactory::make(['username' => 'jane@passbolt.test'])->admin()->persist(); + $jane = UserFactory::make(['username' => 'jane@passbolt.test']) + ->admin() + ->with('Profiles', ['first_name' => 'Jane', 'last_name' => 'Doe']) + ->persist(); $john = UserFactory::make(['username' => 'john@passbolt.test'])->admin()->persist(); $ada = UserFactory::make(['username' => 'ada@passbolt.test'])->admin()->persist(); UserFactory::make()->user()->persist(); @@ -67,10 +70,9 @@ public function testUsersEditAdminRoleRevokedController_NotificationEnabled(): v // Email assertions $this->assertEventFired(UsersEditController::EVENT_USER_AFTER_UPDATE); $this->assertEmailQueueCount(2); - $userFullName = $jane->profile->first_name . ' ' . $jane->profile->last_name; foreach ([$john, $ada] as $admin) { $this->assertEmailInBatchContains( - sprintf('%s\'s admin role has been revoked', $userFullName), + sprintf('%s\'s admin role has been revoked', $jane->profile->full_name), $admin->username, '', false @@ -107,7 +109,10 @@ public function testUsersEditAdminRoleRevokedController_NotificationDisabled(): public function testUsersEditAdminRoleRevokedController_NotifyUserWhoseRoleIsChanged(): void { - $jane = UserFactory::make(['username' => 'jane@passbolt.test'])->admin()->persist(); + $jane = UserFactory::make(['username' => 'jane@passbolt.test']) + ->admin() + ->with('Profiles', ['first_name' => 'Jane', 'last_name' => 'Doe']) + ->persist(); $john = UserFactory::make(['username' => 'john@passbolt.test'])->admin()->persist(); $ada = UserFactory::make(['username' => 'ada@passbolt.test'])->admin()->persist(); UserFactory::make()->user()->persist(); @@ -134,10 +139,9 @@ public function testUsersEditAdminRoleRevokedController_NotifyUserWhoseRoleIsCha Router::url('/app/users/view/' . $jane->id, true), $jane->username ); - $userFullName = $jane->profile->first_name . ' ' . $jane->profile->last_name; foreach ([$john, $ada] as $admin) { $this->assertEmailInBatchContains( - sprintf('%s\'s admin role has been revoked', $userFullName), + sprintf('%s\'s admin role has been revoked', $jane->profile->full_name), $admin->username, '', false From c3a43a8f501e89610a1e027473b240a918296802 Mon Sep 17 00:00:00 2001 From: Crowdin Date: Wed, 8 Nov 2023 20:08:12 +0000 Subject: [PATCH 02/16] New translations cake.po (French) [skip-ci] --- resources/locales/fr_FR/cake.po | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/resources/locales/fr_FR/cake.po b/resources/locales/fr_FR/cake.po index ad86cc408e..b55ee0747b 100644 --- a/resources/locales/fr_FR/cake.po +++ b/resources/locales/fr_FR/cake.po @@ -2,7 +2,7 @@ msgid "" msgstr "" "Project-Id-Version: 41c2572bd9bd4cc908d3e09e0cbed6e5\n" "POT-Creation-Date: 2020-11-11 13:56+0100\n" -"PO-Revision-Date: 2023-09-20 08:32\n" +"PO-Revision-Date: 2023-11-08 20:08\n" "Last-Translator: NAME \n" "Language-Team: French\n" "MIME-Version: 1.0\n" @@ -23,7 +23,7 @@ msgstr "Erreur" #: Error/error400.php:37 msgid "The requested address {0} was not found on this server." -msgstr "" +msgstr "L'adresse demandée {0} est introuvable sur ce serveur." #: Error/error500.php:38 msgid "An Internal Error Has Occurred" @@ -43,11 +43,11 @@ msgstr "Une erreur interne est survenue." #: Http/Middleware/CsrfProtectionMiddleware.php:286 msgid "Missing or incorrect CSRF cookie type." -msgstr "" +msgstr "Le type de cookie CSRF est manquant ou incorrect." #: Http/Middleware/CsrfProtectionMiddleware.php:290 msgid "Missing or invalid CSRF cookie." -msgstr "" +msgstr "Le cookie CSRF est manquant ou incorrect." #: Http/Middleware/CsrfProtectionMiddleware.php:311 msgid "CSRF token from either the request body or request headers did not match or is missing." From c868ae6b74e2acdabf6061109142938a7252417e Mon Sep 17 00:00:00 2001 From: Crowdin Date: Wed, 8 Nov 2023 21:05:39 +0000 Subject: [PATCH 03/16] New translations cake.po (French) [skip-ci] --- resources/locales/fr_FR/cake.po | 94 ++++++++++++++++----------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/resources/locales/fr_FR/cake.po b/resources/locales/fr_FR/cake.po index b55ee0747b..ca81b4a499 100644 --- a/resources/locales/fr_FR/cake.po +++ b/resources/locales/fr_FR/cake.po @@ -2,7 +2,7 @@ msgid "" msgstr "" "Project-Id-Version: 41c2572bd9bd4cc908d3e09e0cbed6e5\n" "POT-Creation-Date: 2020-11-11 13:56+0100\n" -"PO-Revision-Date: 2023-11-08 20:08\n" +"PO-Revision-Date: 2023-11-08 21:05\n" "Last-Translator: NAME \n" "Language-Team: French\n" "MIME-Version: 1.0\n" @@ -35,7 +35,7 @@ msgstr "Vous n'êtes pas autorisé à accéder à cet emplacement." #: Error/ExceptionRenderer.php:304 msgid "Not Found" -msgstr "" +msgstr "Introuvable" #: Error/ExceptionRenderer.php:306 msgid "An Internal Error Has Occurred." @@ -51,37 +51,37 @@ msgstr "Le cookie CSRF est manquant ou incorrect." #: Http/Middleware/CsrfProtectionMiddleware.php:311 msgid "CSRF token from either the request body or request headers did not match or is missing." -msgstr "" +msgstr "Le jeton CSRF du corps ou des en-têtes de la requête ne correspond pas ou est manquant." #: Http/Response.php:1490 msgid "The requested file contains `..` and will not be read." -msgstr "" +msgstr "Le fichier demandé contient `..` et ne sera pas lu." #: Http/Response.php:1498 msgid "The requested file was not found" -msgstr "" +msgstr "Le fichier demandé est introuvable" #: I18n/Number.php:116 msgid "{0,number,#,###.##} KB" -msgstr "" +msgstr "{0,number,#,###.##} Ko" #: I18n/Number.php:118 msgid "{0,number,#,###.##} MB" -msgstr "" +msgstr "{0,number,#,###.##} Mo" #: I18n/Number.php:120 msgid "{0,number,#,###.##} GB" -msgstr "" +msgstr "{0,number,#,###.##} Go" #: I18n/Number.php:122 msgid "{0,number,#,###.##} TB" -msgstr "" +msgstr "{0,number,#,###.##} To" #: I18n/Number.php:114 msgid "{0,number,integer} Byte" msgid_plural "{0,number,integer} Bytes" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0,number,integer} Octet" +msgstr[1] "{0,number,integer} Octets" #: I18n/RelativeTimeFormatter.php:86 msgid "{0} from now" @@ -105,71 +105,71 @@ msgstr "à l'instant" #: I18n/RelativeTimeFormatter.php:157 msgid "about a second ago" -msgstr "" +msgstr "il y a environ une seconde" #: I18n/RelativeTimeFormatter.php:158 msgid "about a minute ago" -msgstr "" +msgstr "il y a environ une minute" #: I18n/RelativeTimeFormatter.php:159 msgid "about an hour ago" -msgstr "" +msgstr "il y a environ une heure" #: I18n/RelativeTimeFormatter.php:160 #: I18n/RelativeTimeFormatter.php:370 msgid "about a day ago" -msgstr "" +msgstr "il y a environ un jour" #: I18n/RelativeTimeFormatter.php:161 #: I18n/RelativeTimeFormatter.php:371 msgid "about a week ago" -msgstr "" +msgstr "il y a environ une semaine" #: I18n/RelativeTimeFormatter.php:162 #: I18n/RelativeTimeFormatter.php:372 msgid "about a month ago" -msgstr "" +msgstr "il y a environ un mois" #: I18n/RelativeTimeFormatter.php:163 #: I18n/RelativeTimeFormatter.php:373 msgid "about a year ago" -msgstr "" +msgstr "il y a environ un an" #: I18n/RelativeTimeFormatter.php:174 msgid "in about a second" -msgstr "" +msgstr "dans environ une seconde" #: I18n/RelativeTimeFormatter.php:175 msgid "in about a minute" -msgstr "" +msgstr "dans environ une minute" #: I18n/RelativeTimeFormatter.php:176 msgid "in about an hour" -msgstr "" +msgstr "dans environ une heure" #: I18n/RelativeTimeFormatter.php:177 #: I18n/RelativeTimeFormatter.php:384 msgid "in about a day" -msgstr "" +msgstr "dans environ un jour" #: I18n/RelativeTimeFormatter.php:178 #: I18n/RelativeTimeFormatter.php:385 msgid "in about a week" -msgstr "" +msgstr "dans environ une semaine" #: I18n/RelativeTimeFormatter.php:179 #: I18n/RelativeTimeFormatter.php:386 msgid "in about a month" -msgstr "" +msgstr "dans environ un mois" #: I18n/RelativeTimeFormatter.php:180 #: I18n/RelativeTimeFormatter.php:387 msgid "in about a year" -msgstr "" +msgstr "dans environ un an" #: I18n/RelativeTimeFormatter.php:342 msgid "today" -msgstr "" +msgstr "aujourd'hui" #: I18n/RelativeTimeFormatter.php:409 msgid "%s ago" @@ -184,65 +184,65 @@ msgstr "" #: I18n/RelativeTimeFormatter.php:354 msgid "{0} year" msgid_plural "{0} years" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} an" +msgstr[1] "{0} ans" #: I18n/RelativeTimeFormatter.php:57 #: I18n/RelativeTimeFormatter.php:135 #: I18n/RelativeTimeFormatter.php:357 msgid "{0} month" msgid_plural "{0} months" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} mois" +msgstr[1] "{0} mois" #: I18n/RelativeTimeFormatter.php:63 #: I18n/RelativeTimeFormatter.php:138 #: I18n/RelativeTimeFormatter.php:360 msgid "{0} week" msgid_plural "{0} weeks" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} semaine" +msgstr[1] "{0} semaines" #: I18n/RelativeTimeFormatter.php:65 #: I18n/RelativeTimeFormatter.php:141 #: I18n/RelativeTimeFormatter.php:363 msgid "{0} day" msgid_plural "{0} days" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} jour" +msgstr[1] "{0} jours" #: I18n/RelativeTimeFormatter.php:70 #: I18n/RelativeTimeFormatter.php:144 msgid "{0} hour" msgid_plural "{0} hours" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} heure" +msgstr[1] "{0} heures" #: I18n/RelativeTimeFormatter.php:74 #: I18n/RelativeTimeFormatter.php:147 msgid "{0} minute" msgid_plural "{0} minutes" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} minute" +msgstr[1] "{0} minutes" #: I18n/RelativeTimeFormatter.php:78 #: I18n/RelativeTimeFormatter.php:150 msgid "{0} second" msgid_plural "{0} seconds" -msgstr[0] "" -msgstr[1] "" +msgstr[0] "{0} seconde" +msgstr[1] "{0} secondes" #: ORM/RulesChecker.php:55 msgid "This value is already in use" -msgstr "" +msgstr "Cette valeur est déjà utilisée" #: ORM/RulesChecker.php:102 msgid "This value does not exist" -msgstr "" +msgstr "Cette valeur n'existe pas" #: ORM/RulesChecker.php:224 msgid "Cannot modify row: a constraint for the `{0}` association fails." -msgstr "" +msgstr "Impossible de modifier la ligne : une contrainte pour l'association `{0}` a échoué." #: ORM/RulesChecker.php:262 msgid "The count does not match {0}{1}" @@ -254,20 +254,20 @@ msgstr "et" #: Validation/Validator.php:2500 msgid "This field is required" -msgstr "" +msgstr "Ce champ est obligatoire" #: Validation/Validator.php:2520 #: View/Form/ArrayContext.php:249 msgid "This field cannot be left empty" -msgstr "" +msgstr "Ce champ ne peut être laissé vide" #: Validation/Validator.php:2672 msgid "The provided value is invalid" -msgstr "" +msgstr "La valeur fournie est incorrecte" #: View/Helper/FormHelper.php:981 msgid "Edit {0}" -msgstr "" +msgstr "Modifier {0}" #: View/Helper/FormHelper.php:983 msgid "New {0}" From 98033ae2f71e841a5c8dabd1c3326edfa011b4f3 Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Thu, 16 Nov 2023 00:57:30 +0100 Subject: [PATCH 04/16] PB-27927 Drops user_agents table --- .../20231115235026_V441DropUserAgents.php | 33 +++++++ src/Model/Entity/UserAgent.php | 28 ------ src/Model/Table/UserAgentsTable.php | 98 ------------------- src/Service/Setup/AbstractStartService.php | 81 --------------- src/Utility/Healthchecks.php | 1 - 5 files changed, 33 insertions(+), 208 deletions(-) create mode 100644 config/Migrations/20231115235026_V441DropUserAgents.php delete mode 100644 src/Model/Entity/UserAgent.php delete mode 100644 src/Model/Table/UserAgentsTable.php delete mode 100644 src/Service/Setup/AbstractStartService.php diff --git a/config/Migrations/20231115235026_V441DropUserAgents.php b/config/Migrations/20231115235026_V441DropUserAgents.php new file mode 100644 index 0000000000..fef6c20af3 --- /dev/null +++ b/config/Migrations/20231115235026_V441DropUserAgents.php @@ -0,0 +1,33 @@ +table('user_agents'); + $table->drop()->save(); + } +} diff --git a/src/Model/Entity/UserAgent.php b/src/Model/Entity/UserAgent.php deleted file mode 100644 index 0e64036d54..0000000000 --- a/src/Model/Entity/UserAgent.php +++ /dev/null @@ -1,28 +0,0 @@ - - */ - protected $_accessible = [ - 'name' => true, - ]; -} diff --git a/src/Model/Table/UserAgentsTable.php b/src/Model/Table/UserAgentsTable.php deleted file mode 100644 index 0c3cfe388b..0000000000 --- a/src/Model/Table/UserAgentsTable.php +++ /dev/null @@ -1,98 +0,0 @@ -setTable('user_agents'); - $this->setDisplayField('name'); - $this->setPrimaryKey('id'); - } - - /** - * Default validation rules. - * - * @param \Cake\Validation\Validator $validator Validator instance. - * @return \Cake\Validation\Validator - */ - public function validationDefault(Validator $validator): Validator - { - $validator - ->uuid('id', __('The identifier should be a valid UUID.')) - ->allowEmptyString('id', __('The identifier should not be empty.'), 'create'); - - $validator - ->utf8('name', __('The name should be a valid BMP-UTF8 string.')) - ->allowEmptyString('name'); - - return $validator; - } - - /** - * Sanitize and parse the user agent string - * - * @param string|null $ua user agent (optional) - * @return string|null - */ - public function browserName(?string $ua = null): ?string - { - if ($ua == null) { - $ua = Purifier::clean(env('HTTP_USER_AGENT')); - } - $browserName = 'undefined'; - try { - $provider = new UserAgentParser(); - $userAgent = $provider->parse($ua); - $browserName = $userAgent->browser(); - } catch (\Exception $e) { - } - - return $browserName; - } -} diff --git a/src/Service/Setup/AbstractStartService.php b/src/Service/Setup/AbstractStartService.php deleted file mode 100644 index 6fc58424f9..0000000000 --- a/src/Service/Setup/AbstractStartService.php +++ /dev/null @@ -1,81 +0,0 @@ -AuthenticationTokens = $this->fetchTable('AuthenticationTokens'); - /** @phpstan-ignore-next-line */ - $this->UserAgents = $this->fetchTable('UserAgents'); - /** @phpstan-ignore-next-line */ - $this->Users = $this->fetchTable('Users'); - $this->request = $request ?? new ServerRequest(); - } - - /** - * Assert if the browser is supported. Redirect if the browser is not supported. - * - * @return bool - */ - protected function isBrowserSupported(): bool - { - $supportedBrowsers = ['firefox', 'chrome']; - $browserName = strtolower($this->UserAgents->browserName()); - if (!in_array($browserName, $supportedBrowsers)) { - return false; - } - - return true; - } -} diff --git a/src/Utility/Healthchecks.php b/src/Utility/Healthchecks.php index 47e1add0f7..e11b57a2f3 100644 --- a/src/Utility/Healthchecks.php +++ b/src/Utility/Healthchecks.php @@ -387,7 +387,6 @@ public static function getSchemaTables(int $version = 2): array 'resources', 'roles', 'secrets', - 'user_agents', 'users', ]; From d26d8f995de18c20c0a9a422624e40f174f182fe Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Thu, 16 Nov 2023 04:25:47 +0100 Subject: [PATCH 05/16] PB-27957 Send an email only if an admin has been deleted --- .../User/AdminDeleteEmailRedactor.php | 3 ++ .../AdminDeleteNotificationTest.php | 36 ++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Notification/Email/Redactor/User/AdminDeleteEmailRedactor.php b/src/Notification/Email/Redactor/User/AdminDeleteEmailRedactor.php index a7764949ff..ed46015353 100644 --- a/src/Notification/Email/Redactor/User/AdminDeleteEmailRedactor.php +++ b/src/Notification/Email/Redactor/User/AdminDeleteEmailRedactor.php @@ -89,6 +89,9 @@ public function onSubscribedEvent(Event $event): EmailCollection if (!$deletedUser instanceof User) { throw new InvalidArgumentException('`user` is missing from event data.'); } + if (!$deletedUser->role->isAdmin()) { + return $emailCollection; + } /** @var array $groupsIds */ $groupsIds = $event->getData('groupsIds'); diff --git a/tests/TestCase/Controller/Notifications/AdminDeleteNotificationTest.php b/tests/TestCase/Controller/Notifications/AdminDeleteNotificationTest.php index de8a372c3b..7d33908a70 100644 --- a/tests/TestCase/Controller/Notifications/AdminDeleteNotificationTest.php +++ b/tests/TestCase/Controller/Notifications/AdminDeleteNotificationTest.php @@ -48,39 +48,51 @@ public function setUp(): void public function testAdminDeleteNotification_Success(): void { - /** @var \App\Model\Entity\User $admin */ - $admin = UserFactory::make()->admin()->active()->persist(); - /** @var \App\Model\Entity\User $operator */ - $operator = UserFactory::make()->admin()->active()->persist(); - /** @var \App\Model\Entity\User $johnAdmin */ - $johnAdmin = UserFactory::make()->admin()->active()->persist(); + [$adminDeleted, $operator, $otherAdmin] = UserFactory::make(3)->admin()->persist(); + // This admin should not receive notifications + UserFactory::make()->admin()->disabled()->persist(); + // This user should not receive notifications + UserFactory::make()->user()->persist(); $this->logInAs($operator); - $this->deleteJson("/users/{$admin->id}.json"); + $this->deleteJson("/users/{$adminDeleted->id}.json"); $this->assertSuccess(); // Email assertions $this->assertEmailQueueCount(2); - $adminFullName = $admin->profile->full_name; + $adminFullName = $adminDeleted->profile->full_name; $operatorFullName = $operator->profile->full_name; $this->assertEmailInBatchContains( "You deleted administrator {$adminFullName}", $operator->username ); $this->assertEmailInBatchContains( - "The administrator {$adminFullName} ({$admin->username}) is now deleted from the passbolt organisation.", + "The administrator {$adminFullName} ({$adminDeleted->username}) is now deleted from the passbolt organisation.", $operator->username ); $this->assertEmailInBatchContains( "{$operatorFullName} deleted administrator {$adminFullName}", - $johnAdmin->username + $otherAdmin->username ); $this->assertEmailInBatchContains( - "The administrator {$adminFullName} ({$admin->username}) is now deleted from the passbolt organisation.", - $johnAdmin->username + "The administrator {$adminFullName} ({$adminDeleted->username}) is now deleted from the passbolt organisation.", + $otherAdmin->username ); } + public function testAdminDeleteNotification_Delete_User_Should_Not_Send_Notification(): void + { + /** @var \App\Model\Entity\User $userDeleted */ + $userDeleted = UserFactory::make()->user()->persist(); + + $this->logInAsAdmin(); + $this->deleteJson("/users/{$userDeleted->id}.json"); + + $this->assertSuccess(); + // No emails should be sent if the deleted user is not an admin + $this->assertEmailQueueCount(0); + } + public function testAdminDeleteNotification_NotificationOff(): void { /** @var \App\Model\Entity\User $admin */ From bd6799b41eea04da15b273e7b4b453218b65485e Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Thu, 16 Nov 2023 15:13:59 +0100 Subject: [PATCH 06/16] PB-27927 Fixes MigratePostgresCommand --- ...1231300_V340MigrateASCIIFieldsEncoding.php | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/config/Migrations/20211121231300_V340MigrateASCIIFieldsEncoding.php b/config/Migrations/20211121231300_V340MigrateASCIIFieldsEncoding.php index cd33c5d1ec..b1f897b5ae 100644 --- a/config/Migrations/20211121231300_V340MigrateASCIIFieldsEncoding.php +++ b/config/Migrations/20211121231300_V340MigrateASCIIFieldsEncoding.php @@ -599,15 +599,17 @@ public function up() ]) ->save(); - $this->table('user_agents') - ->changeColumn('id', 'uuid', [ - 'default' => null, - 'limit' => null, - 'null' => false, - 'encoding' => 'ascii', - 'collation' => 'ascii_general_ci' - ]) - ->save(); + if ($this->hasTable('user_agents')) { + $this->table('user_agents') + ->changeColumn('id', 'uuid', [ + 'default' => null, + 'limit' => null, + 'null' => false, + 'encoding' => 'ascii', + 'collation' => 'ascii_general_ci' + ]) + ->save(); + } $this->table('users') ->changeColumn('id', 'uuid', [ From 2f70d0d6c008122ac7f150ef2c907da0183b7352 Mon Sep 17 00:00:00 2001 From: jpramirez Date: Fri, 17 Nov 2023 08:33:34 +0000 Subject: [PATCH 07/16] PB-28616 Refactor email digest --- phpstan-baseline.neon | 10 - plugins/PassboltCe/EmailDigest/README.md | 71 +++ .../src/Command/PreviewCommand.php | 19 +- .../EmailDigest/src/Command/SenderCommand.php | 9 +- .../EmailDigest/src/EmailDigestPlugin.php | 20 +- .../src/Service/EmailDigestService.php | 32 +- .../src/Service/PreviewEmailBatchService.php | 69 +-- .../src/Service/SendEmailBatchService.php | 22 +- .../src/Utility/Digest/AbstractDigest.php | 78 ---- .../Utility/Digest/AbstractDigestRegister.php | 55 --- .../Utility/Digest/AbstractDigestTemplate.php | 133 ++++++ .../EmailDigest/src/Utility/Digest/Digest.php | 302 +++++------- .../Utility/Digest/DigestRegisterEvent.php | 62 --- .../Utility/Digest/DigestTemplateRegistry.php | 98 ++++ .../src/Utility/Digest/DigestsCollection.php | 98 ---- .../src/Utility/Digest/DigestsPool.php | 117 ----- .../src/Utility/Digest/SingleDigest.php | 89 ---- .../AbstractDigestCollection.php} | 22 +- .../DigestCollection/DigestsCollection.php | 184 ++++++++ .../SingleDigestCollection.php | 63 +++ .../src/Utility/Factory/DigestFactory.php | 109 ----- .../Utility/Factory/EmailPreviewFactory.php | 80 +++- .../src/Utility/Mailer/EmailDigest.php | 125 ++--- .../Utility/Mailer/EmailDigestInterface.php | 20 +- .../src/Utility/Mailer/EmailInterface.php | 38 +- .../tests/Factory/EmailQueueFactory.php | 38 +- .../GroupUserDeleteEmailQueueFactory.php | 56 +++ .../ResourceDeleteEmailQueueFactory.php | 65 +++ .../tests/Lib/EmailDigestMockTestTrait.php | 109 ----- .../TestCase/Command/PreviewCommandTest.php | 5 +- .../TestCase/Command/SenderCommandTest.php | 26 +- .../Service/PreviewEmailBatchServiceTest.php | 17 +- ...ServiceCreateResourceChangesDigestTest.php | 16 +- .../Service/SendEmailBatchServiceTest.php | 225 --------- .../Service/SendEmailBatchServiceUnitTest.php | 442 ++++++++++++++++++ .../Utility/Digest/DigestsCollectionTest.php | 165 ------- .../Utility/Digest/DigestsPoolTest.php | 72 --- .../Utility/Digest/SingleEmailDigestTest.php | 84 ---- psalm-baseline-v5-upgrade.xml | 27 -- .../GroupMembershipDigestTemplate.php | 72 +++ .../GroupUserDeleteDigestTemplate.php | 68 +++ .../ResourceChangesDigestTemplate.php | 76 +++ .../ResourceShareDigestTemplate.php | 71 +++ .../Group/GroupUserDeleteEmailRedactor.php | 7 +- .../Resource/ResourceDeleteEmailRedactor.php | 8 +- .../DigestRegister/GroupDigests.php | 121 ----- .../DigestRegister/ResourceDigests.php | 148 ------ templates/email/html/LU/group_user_delete.php | 2 +- .../email/html/LU/group_users_change.php | 3 +- templates/email/html/LU/groups_delete.php | 7 +- templates/email/html/LU/resource_delete.php | 3 +- templates/email/html/LU/resources_change.php | 3 +- tests/Factory/UserFactory.php | 10 + tests/Lib/AppIntegrationTestCase.php | 6 +- tests/Lib/AppTestCase.php | 6 +- tests/Lib/SolutionBootstrapperTestCase.php | 4 + tests/Lib/Utility/EmailTestTrait.php | 4 +- 57 files changed, 1853 insertions(+), 2038 deletions(-) create mode 100644 plugins/PassboltCe/EmailDigest/README.md delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigest.php delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestRegister.php create mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestTemplate.php delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestRegisterEvent.php create mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestTemplateRegistry.php delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsCollection.php delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsPool.php delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Digest/SingleDigest.php rename plugins/PassboltCe/EmailDigest/src/Utility/{Digest/DigestInterface.php => DigestCollection/AbstractDigestCollection.php} (68%) create mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/DigestsCollection.php create mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/SingleDigestCollection.php delete mode 100644 plugins/PassboltCe/EmailDigest/src/Utility/Factory/DigestFactory.php create mode 100644 plugins/PassboltCe/EmailDigest/tests/Factory/GroupUserDeleteEmailQueueFactory.php create mode 100644 plugins/PassboltCe/EmailDigest/tests/Factory/ResourceDeleteEmailQueueFactory.php delete mode 100644 plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceTest.php create mode 100644 plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceUnitTest.php delete mode 100644 plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsCollectionTest.php delete mode 100644 plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsPoolTest.php delete mode 100644 plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/SingleEmailDigestTest.php create mode 100644 src/Notification/DigestTemplate/GroupMembershipDigestTemplate.php create mode 100644 src/Notification/DigestTemplate/GroupUserDeleteDigestTemplate.php create mode 100644 src/Notification/DigestTemplate/ResourceChangesDigestTemplate.php create mode 100644 src/Notification/DigestTemplate/ResourceShareDigestTemplate.php delete mode 100644 src/Notification/EmailDigest/DigestRegister/GroupDigests.php delete mode 100644 src/Notification/EmailDigest/DigestRegister/ResourceDigests.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 66e9a9d752..f7662bf61e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -5,16 +5,6 @@ parameters: count: 1 path: plugins/PassboltCe/AccountSettings/src/Model/Table/AccountSettingsTable.php - - - message: "#^Unsafe access to private property Passbolt\\\\EmailDigest\\\\Utility\\\\Digest\\\\DigestsPool\\:\\:\\$instance through static\\:\\:\\.$#" - count: 4 - path: plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsPool.php - - - - message: "#^Unsafe access to private property Passbolt\\\\EmailDigest\\\\Utility\\\\Factory\\\\DigestFactory\\:\\:\\$instance through static\\:\\:\\.$#" - count: 4 - path: plugins/PassboltCe/EmailDigest/src/Utility/Factory/DigestFactory.php - - message: "#^Unsafe access to private property Passbolt\\\\EmailNotificationSettings\\\\Utility\\\\EmailNotificationSettings\\:\\:\\$configSettingsSource through static\\:\\:\\.$#" count: 4 diff --git a/plugins/PassboltCe/EmailDigest/README.md b/plugins/PassboltCe/EmailDigest/README.md new file mode 100644 index 0000000000..5786b7c613 --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/README.md @@ -0,0 +1,71 @@ +# Introduction +After importing a large amount of resources, or any action generating a large amount of emails, +we would like the emails to be grouped by operator and template into one single email resuming +the action performed. + +When the number of emails ina digest is below or equal 10, all the emails are rendered +within the email digest. + +When the number of emails under a digest is above 10 emails, only a summary email is sent. + +This document describes the approach taken. + +# How to add a digest template + +Typically, the digest templates are added in the plugins, e.g. in EmailDigestPlugin for the default email digests, +using the `DigestTemplateRegistry`: + +```php +DigestTemplateRegistry::getInstance() + ->addTemplate(new ResourceChangesDigestTemplate(5)) + ->addTemplate(new ResourceShareDigestTemplate(1)) + ... +``` +The priority of the digest template may be passed in the constructor. Default is 10, the highest priority is 1. + + + +# Technical specifications + +This section describes the various components involved in the sending of email digest. + +## `DigestTemplateRegistry` + +This class gathers all the digest templates. Typically, it is used in the `EmailDigestPlugin::bootstrap();` +to register which digest templates are available. Note that templates are added in when performing an action +at the command line only. + +## `AbstractDigestTemplate` + +A digest template defines the email templates that will be covered by a digest, the priority +of a digest, the template used by this digest when the number of emails is above 10. + +Default priority is 10, and a number below enhances the priority. So priority 1 is higher than priority 2. + +You may follow the abstract methods of this abstract template to build a new digest template. + +## `DigestsCollection` + +When sending or previewing a batch of emails (per default 100, see `PASSBOLT_PLUGINS_EMAIL_DIGEST_BATCH_SIZE_LIMIT`), +the emails are attributed to a digest, if they are candidates to a digest, or will be +rendered as single emails. The `DigestsCollection` iterates through all the emails +to be sent and creates digests resp. assigns to these digests the emails candidate. + +## `Digest` +A digest contains the emails for: +- a single operator +- a single recipient +- a single digest template + +The `marshalEmails` method renders the emails within the digest, returning an `EmailDigest`. + +## `EmailDigest` +This class is the conversion of a `Digest` into an object that can be sent as an email. It is the email realization +of a digest, with all the information needed to create an email to be sent. + +# Summary +The developer registers `AbstractDigestTemplate`s. When emails are sent, these are grouped in `Digest`s of the same +recipient and operator by the `DigestsCollection`, or left as single emails if they do not find a matching digest template. + +When sending the emails, each `Digest` is previewed resp. sent thanks to the `EmailDigest` class. + diff --git a/plugins/PassboltCe/EmailDigest/src/Command/PreviewCommand.php b/plugins/PassboltCe/EmailDigest/src/Command/PreviewCommand.php index 990f511468..a83f764293 100644 --- a/plugins/PassboltCe/EmailDigest/src/Command/PreviewCommand.php +++ b/plugins/PassboltCe/EmailDigest/src/Command/PreviewCommand.php @@ -22,6 +22,8 @@ use Cake\Console\ConsoleIo; use Cake\Console\ConsoleOptionParser; use Cake\Core\Configure; +use Cake\ORM\TableRegistry; +use Cake\Utility\Hash; use Passbolt\EmailDigest\Service\PreviewEmailBatchService; class PreviewCommand extends PassboltCommand @@ -57,10 +59,21 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar */ public function execute(Arguments $args, ConsoleIo $io): ?int { - $emailSenderService = new PreviewEmailBatchService(); - $limit = (int)$args->getOption('limit'); - $previews = $emailSenderService->previewNextEmailsBatch($limit); + /** @var \EmailQueue\Model\Table\EmailQueueTable $EmailQueueTable */ + $EmailQueueTable = TableRegistry::getTableLocator()->get('EmailQueue.EmailQueue'); + $emailQueues = $EmailQueueTable->getBatch($limit); + + Configure::write('App.baseUrl', '/'); + + if (!empty($emailQueues)) { + // we release the locks as soon as we get the emails + // we don't want to block the next batch ran by a cron job because of lock. + // technically, to do better, we should write the same query ran in getBatch method without locking the emails + $EmailQueueTable->releaseLocks(Hash::extract($emailQueues, '{n}.id')); + } + + $previews = (new PreviewEmailBatchService())->previewNextEmailsBatch($emailQueues); foreach ($previews as $preview) { $io->out($preview->getHeaders()); if ($args->getOption('body') === true) { diff --git a/plugins/PassboltCe/EmailDigest/src/Command/SenderCommand.php b/plugins/PassboltCe/EmailDigest/src/Command/SenderCommand.php index 137ad35c2f..f6f48f313a 100644 --- a/plugins/PassboltCe/EmailDigest/src/Command/SenderCommand.php +++ b/plugins/PassboltCe/EmailDigest/src/Command/SenderCommand.php @@ -22,6 +22,7 @@ use Cake\Console\ConsoleIo; use Cake\Console\ConsoleOptionParser; use Cake\Core\Configure; +use Cake\ORM\TableRegistry; use Passbolt\EmailDigest\Service\SendEmailBatchService; class SenderCommand extends PassboltCommand @@ -47,9 +48,11 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar */ public function execute(Arguments $args, ConsoleIo $io): ?int { - $emailSenderService = new SendEmailBatchService(); - - $emailSenderService->sendNextEmailsBatch((int)$args->getOption('limit')); + $limit = (int)$args->getOption('limit'); + /** @var \EmailQueue\Model\Table\EmailQueueTable $EmailQueueTable */ + $EmailQueueTable = TableRegistry::getTableLocator()->get('EmailQueue.EmailQueue'); + $emails = $EmailQueueTable->getBatch($limit); + (new SendEmailBatchService())->sendNextEmailsBatch($emails); return $this->successCode(); } diff --git a/plugins/PassboltCe/EmailDigest/src/EmailDigestPlugin.php b/plugins/PassboltCe/EmailDigest/src/EmailDigestPlugin.php index 627c3db0d7..930f5f059c 100644 --- a/plugins/PassboltCe/EmailDigest/src/EmailDigestPlugin.php +++ b/plugins/PassboltCe/EmailDigest/src/EmailDigestPlugin.php @@ -16,14 +16,16 @@ */ namespace Passbolt\EmailDigest; -use App\Notification\EmailDigest\DigestRegister\GroupDigests; -use App\Notification\EmailDigest\DigestRegister\ResourceDigests; +use App\Notification\DigestTemplate\GroupMembershipDigestTemplate; +use App\Notification\DigestTemplate\GroupUserDeleteDigestTemplate; +use App\Notification\DigestTemplate\ResourceChangesDigestTemplate; +use App\Notification\DigestTemplate\ResourceShareDigestTemplate; use Cake\Console\CommandCollection; use Cake\Core\BasePlugin; -use Cake\Core\Configure; use Cake\Core\PluginApplicationInterface; use Passbolt\EmailDigest\Command\PreviewCommand; use Passbolt\EmailDigest\Command\SenderCommand; +use Passbolt\EmailDigest\Utility\Digest\DigestTemplateRegistry; class EmailDigestPlugin extends BasePlugin { @@ -34,12 +36,12 @@ public function bootstrap(PluginApplicationInterface $app): void { parent::bootstrap($app); - if (PHP_SAPI === 'cli' || (Configure::read('debug') && Configure::read('passbolt.selenium.active'))) { - // Core email digests - $app->getEventManager() - ->on(new GroupDigests()) - ->on(new ResourceDigests()); - } + // Core email digests + DigestTemplateRegistry::getInstance() + ->addTemplate(new ResourceChangesDigestTemplate()) + ->addTemplate(new ResourceShareDigestTemplate()) + ->addTemplate(new GroupUserDeleteDigestTemplate()) + ->addTemplate(new GroupMembershipDigestTemplate()); } /** diff --git a/plugins/PassboltCe/EmailDigest/src/Service/EmailDigestService.php b/plugins/PassboltCe/EmailDigest/src/Service/EmailDigestService.php index 901fd9d7e4..544b86124d 100644 --- a/plugins/PassboltCe/EmailDigest/src/Service/EmailDigestService.php +++ b/plugins/PassboltCe/EmailDigest/src/Service/EmailDigestService.php @@ -17,7 +17,9 @@ namespace Passbolt\EmailDigest\Service; -use Passbolt\EmailDigest\Utility\Factory\DigestFactory; +use Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException; +use Passbolt\EmailDigest\Utility\DigestCollection\DigestsCollection; +use Passbolt\EmailDigest\Utility\DigestCollection\SingleDigestCollection; /** * The EmailDigestService is a service designed to aggregate multiple emails together. @@ -37,19 +39,6 @@ */ class EmailDigestService { - /** - * @var \Passbolt\EmailDigest\Utility\Factory\DigestFactory - */ - private $digestFactory; - - /** - * @param \Passbolt\EmailDigest\Utility\Factory\DigestFactory|null $digestFactory Factory - */ - public function __construct(?DigestFactory $digestFactory = null) - { - $this->digestFactory = $digestFactory ?? DigestFactory::getInstance(); - } - /** * Handle the emails data for each recipient and transform them to emails digests * @@ -57,20 +46,21 @@ public function __construct(?DigestFactory $digestFactory = null) * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException */ - public function createDigests(array $emails): array + public function createEmailDigests(array $emails): array { // Group the emails entities by recipient and create digests for each group of emails. - $digestsCollection = $this->digestFactory->createDigestsCollection(); - $fallback = $this->digestFactory->createSingleDigest(); + $digestsCollection = new DigestsCollection(); + // Collect all the emails that are not covered by an email digest + $singleDigestCollection = new SingleDigestCollection(); foreach ($emails as $emailQueueEntity) { - if ($digestsCollection->canAddToDigest($emailQueueEntity)) { + try { $digestsCollection->addEmailEntity($emailQueueEntity); - } else { - $fallback->addEmailEntity($emailQueueEntity); + } catch (UnsupportedEmailDigestDataException $exception) { + $singleDigestCollection->addEmailEntity($emailQueueEntity); } } - return array_merge($fallback->marshalEmails(), $digestsCollection->marshalEmails()); + return array_merge($singleDigestCollection->marshalEmails(), $digestsCollection->marshalEmails()); } } diff --git a/plugins/PassboltCe/EmailDigest/src/Service/PreviewEmailBatchService.php b/plugins/PassboltCe/EmailDigest/src/Service/PreviewEmailBatchService.php index d25c88ce71..a943546156 100644 --- a/plugins/PassboltCe/EmailDigest/src/Service/PreviewEmailBatchService.php +++ b/plugins/PassboltCe/EmailDigest/src/Service/PreviewEmailBatchService.php @@ -17,10 +17,6 @@ namespace Passbolt\EmailDigest\Service; -use Cake\Core\Configure; -use Cake\ORM\TableRegistry; -use Cake\Utility\Hash; -use EmailQueue\Model\Table\EmailQueueTable; use Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory; /** @@ -31,73 +27,20 @@ */ class PreviewEmailBatchService { - /** - * @var \EmailQueue\Model\Table\EmailQueueTable - */ - private $emailQueueTable; - - /** - * @var \Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory - */ - private $emailPreviewFactory; - - /** - * @var \Passbolt\EmailDigest\Service\EmailDigestService - */ - private $emailDigestsService; - - /** - * @param \EmailQueue\Model\Table\EmailQueueTable|null $emailQueueTable Email Queue Table - * @param \Passbolt\EmailDigest\Service\EmailDigestService|null $emailDigestsService Emails Digests Service - * @param \Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory|null $emailPreviewFactory Email Preview Factory - */ - public function __construct( - ?EmailQueueTable $emailQueueTable = null, - ?EmailDigestService $emailDigestsService = null, - ?EmailPreviewFactory $emailPreviewFactory = null - ) { - $this->emailQueueTable = $emailQueueTable ?? TableRegistry::getTableLocator()->get('EmailQueue.EmailQueue'); - $this->emailDigestsService = $emailDigestsService ?? new EmailDigestService(); - $this->emailPreviewFactory = $emailPreviewFactory ?? new EmailPreviewFactory(); - } - - /** - * Get and send the next emails batch from the email queue. The size of the email batch is determined by $limit. - * - * @param int $limit Size of the emails batch. - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailPreview[] - * @throws \Exception - */ - public function previewNextEmailsBatch(int $limit = 10): array - { - Configure::write('App.baseUrl', '/'); - - $emails = $this->emailQueueTable->getBatch($limit); - - if (!empty($emails)) { - // we release the locks as soon as we get the emails - // we dont want to block the next batch ran by a cron job because of lock. - // technically, to do better, we should write the same query ran in getBatch method without locking the emails - $this->emailQueueTable->releaseLocks(Hash::extract($emails, '{n}.id')); - } - - return $this->getPreviewsOfEmailsAsDigests($emails); - } - /** * Preview a collection of emails as emails digests * - * @param array $emails An array of emails entities + * @param \Cake\ORM\Entity[] $emailQueues array of emails. * @return \Passbolt\EmailDigest\Utility\Mailer\EmailPreview[] - * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException + * @throws \Exception */ - public function getPreviewsOfEmailsAsDigests(array $emails): array + public function previewNextEmailsBatch(array $emailQueues): array { - $emailDigests = $this->emailDigestsService->createDigests($emails); + $emailDigests = (new EmailDigestService())->createEmailDigests($emailQueues); $previews = []; - foreach ($emailDigests as $digest) { - $previews[] = $this->emailPreviewFactory->renderEmailPreviewFromDigest($digest, 'default'); + foreach ($emailDigests as $emailDigest) { + $previews[] = (new EmailPreviewFactory())->renderEmailPreviewFromDigest($emailDigest, 'default'); } return $previews; diff --git a/plugins/PassboltCe/EmailDigest/src/Service/SendEmailBatchService.php b/plugins/PassboltCe/EmailDigest/src/Service/SendEmailBatchService.php index baaf8f5e7f..f04668ae5b 100644 --- a/plugins/PassboltCe/EmailDigest/src/Service/SendEmailBatchService.php +++ b/plugins/PassboltCe/EmailDigest/src/Service/SendEmailBatchService.php @@ -22,7 +22,6 @@ use Cake\Network\Exception\SocketException; use Cake\ORM\TableRegistry; use Cake\TestSuite\EmailTrait; -use EmailQueue\Model\Table\EmailQueueTable; use Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface; /** @@ -41,34 +40,25 @@ class SendEmailBatchService private $emailQueueTable; /** - * @var \Passbolt\EmailDigest\Service\EmailDigestService + * SendEmailBatchService construct */ - private $emailDigestService; - - /** - * @param \EmailQueue\Model\Table\EmailQueueTable|null $table An instance of EmailQueueTable - * @param \Passbolt\EmailDigest\Service\EmailDigestService|null $emailDigestService digest service - */ - public function __construct(?EmailQueueTable $table = null, ?EmailDigestService $emailDigestService = null) + public function __construct() { - $this->emailQueueTable = $table ?? TableRegistry::getTableLocator()->get('EmailQueue.EmailQueue'); - $this->emailDigestService = $emailDigestService ?? new EmailDigestService(); + $this->emailQueueTable = TableRegistry::getTableLocator()->get('EmailQueue.EmailQueue'); } /** * Get and send the next emails batch from the email queue. The size of the email batch is determined by $limit. * - * @param int $limit Size of the emails batch. + * @param \Cake\ORM\Entity[] $emailQueues array of emails. * @return void * @throws \Exception */ - public function sendNextEmailsBatch(int $limit = 10): void + public function sendNextEmailsBatch(array $emailQueues): void { Configure::write('App.baseUrl', '/'); - $emails = $this->emailQueueTable->getBatch($limit); - - $emailDigests = $this->emailDigestService->createDigests($emails); + $emailDigests = (new EmailDigestService())->createEmailDigests($emailQueues); foreach ($emailDigests as $digest) { $this->sendDigest($digest); diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigest.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigest.php deleted file mode 100644 index 0d0fd811fa..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigest.php +++ /dev/null @@ -1,78 +0,0 @@ -emailsData[] = $emailQueueEntity; - - return $this; - } - - /** - * Helper method which render the content of every emails contained in a digest into a string to be used - * as the content of the digest. - * - * @param \Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory $factory Factory - * @param \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface $digest Email digest to use to render - * @return string - */ - protected function renderDigestContentFromEmailPreview( - EmailPreviewFactory $factory, - EmailDigestInterface $digest - ): string { - $emailDigestContent = []; - foreach ($digest->getEmailsData() as $emailData) { - $emailDigestContent[] = $factory->renderFromEmailEntity($emailData); - } - - return implode('', $emailDigestContent); - } - - /** - * @param \Cake\ORM\Entity $emailQueueEntity entity - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigest - */ - protected function buildSingleEmailDigest(Entity $emailQueueEntity): EmailDigest - { - return (new EmailDigest()) - ->addEmailData($emailQueueEntity) - ->setSubject($emailQueueEntity->get('subject')) - ->setEmailRecipient($emailQueueEntity->get('email')); - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestRegister.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestRegister.php deleted file mode 100644 index 0dc4bb9275..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestRegister.php +++ /dev/null @@ -1,55 +0,0 @@ - - */ - public function implementedEvents(): array - { - return [ - DigestRegisterEvent::EVENT_NAME => $this, - ]; - } - - /** - * A class registering digests must implement this method - * - * @param \Passbolt\EmailDigest\Utility\Digest\DigestsPool $digestsPool digest pool - * @return void - */ - abstract public function addDigestsPool(DigestsPool $digestsPool): void; - - /** - * @param \Passbolt\EmailDigest\Utility\Digest\DigestRegisterEvent $event An instance of the event - * @return void - */ - public function __invoke(DigestRegisterEvent $event) - { - $this->addDigestsPool($event->getEmailDigestsPool()); - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestTemplate.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestTemplate.php new file mode 100644 index 0000000000..d7287fbabc --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/AbstractDigestTemplate.php @@ -0,0 +1,133 @@ +priority = $priority; + } + + /** + * Defines the priority of a digest to be rendered over the others + * + * @return int + */ + public function getPriority(): int + { + return $this->priority; + } + + /** + * @param \Passbolt\EmailDigest\Utility\Digest\Digest $digest digest + * @return string + */ + final public function getTranslatedSubject(Digest $digest): string + { + if ($digest->isRecipientTheOperator()) { + $makeSubject = function (): string { + return __($this->getDigestSubjectIfRecipientIsTheOperator()); + }; + } else { + $operatorProfile = $digest->getOperator()->profile; + $operatorFullName = Purifier::clean( + $operatorProfile['first_name'] . ' ' . $operatorProfile['last_name'] + ); + $makeSubject = function () use ($operatorFullName): string { + return __($this->getDigestSubjectIfRecipientIsNotTheOperator(), $operatorFullName); + }; + } + $emailLocale = $digest->getLocale(); + if ($emailLocale !== null) { + $subject = (new LocaleService())->translateString($emailLocale, $makeSubject); + } else { + $subject = $makeSubject(); + } + + return $subject; + } + + /** + * @return string + */ + protected function logErrorIfTheRecipientCannotBeTheOperator(): string + { + Log::error('The recipient cannot be the operator in this email digest template: ' . self::class); + + return $this->getDigestSubjectIfRecipientIsNotTheOperator(); + } +} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/Digest.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/Digest.php index 76d1471217..33db6f972d 100644 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/Digest.php +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/Digest.php @@ -16,291 +16,213 @@ */ namespace Passbolt\EmailDigest\Utility\Digest; -use App\Model\Validation\EmailValidationRule; +use App\Model\Entity\User; +use Cake\Log\Log; use Cake\ORM\Entity; -use Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException; +use Cake\Utility\Hash; use Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory; -use Passbolt\EmailDigest\Utility\Mailer\EmailDigest; -use Passbolt\Locale\Event\LocaleEmailQueueListener; -use Passbolt\Locale\Service\LocaleService; +use Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface; /** - * Default digest to fall back to building a single email - * Adding more than one email to the digest will return as many "digests" as there is emails. + * Digest gathering all emails for one recipient, one operator, and one digest template */ -class Digest extends AbstractDigest implements DigestInterface +class Digest { public const MAXIMUM_EMAILS_IN_DIGEST = 10; /** - * @var \Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory - */ - private $emailPreviewFactory; - - /** - * @var array - */ - private $emails; - - /** - * @var array + * Emails queues gathered in this digest + * + * @var \Cake\ORM\Entity[] */ - private $emailCount; + private array $emailQueues = []; /** - * List of supported templates. An empty list means that all templates are supported. - * Email with a template included in this list will be part of the same digest. - * - * @var string[] + * @var string */ - private $supportedTemplates; + private string $recipient; /** - * Name of the variable in template vars body which contain the user. + * Operator execuring the action * i.e. in template, $body['user'] contains the user executing the action, then $executedByTemplateVarKey = 'user'; * in template, if $body['admin'] contains the user executing the action, then $executedByTemplateVarKey = 'admin' * - * @var string + * @var \App\Model\Entity\User */ - private $executedByTemplateVarKey; + private User $operator; /** - * Subject of the digest. - * - * @var string + * @var \Passbolt\EmailDigest\Utility\Digest\AbstractDigestTemplate */ - private $subject; + private AbstractDigestTemplate $template; /** - * @var callable + * In case the emails queue are shared across various organizations, + * each digest are responsible for one single full base URL + * + * @var string */ - private $onThresholdCallback; + private string $fullBaseUrl; /** * Digest constructor. * - * @param string $subject templates - * @param array $supportedTemplates subject - * @param string $executedByTemplateVarKey key to look for user info in email data - * @param callable $onThresholdCallback what to do when too many emails are present in digest - * @param \Passbolt\EmailDigest\Utility\Factory\EmailPreviewFactory|null $emailPreviewFactory email preview factory + * @param string $recipient digest recipient + * @param \App\Model\Entity\User $operator operator + * @param string $fullBaseUrl full base url + * @param \Cake\ORM\Entity $emailQueue first email queue passed into the digest. A digest cannot be empty + * @param \Passbolt\EmailDigest\Utility\Digest\AbstractDigestTemplate $template digest template used to render this digest */ public function __construct( - string $subject, - array $supportedTemplates, - string $executedByTemplateVarKey, - callable $onThresholdCallback, - ?EmailPreviewFactory $emailPreviewFactory = null + string $recipient, + User $operator, + string $fullBaseUrl, + Entity $emailQueue, + AbstractDigestTemplate $template ) { - $this->supportedTemplates = $supportedTemplates; - $this->subject = $subject; - $this->executedByTemplateVarKey = $executedByTemplateVarKey; - $this->onThresholdCallback = $onThresholdCallback; - $this->emailPreviewFactory = $emailPreviewFactory ?? new EmailPreviewFactory(); - $this->emails = []; - $this->emailCount = []; + $this->recipient = $recipient; + $this->operator = $operator; + $this->fullBaseUrl = $fullBaseUrl; + $this->template = $template; + $this->addEmail($emailQueue); } /** - * Handle a collection of digests internally. Each time a new email is added,, - * it checks if a digest in the collection already exists with the same template than the one from the email to add. - * If it exists, the email will be part of the same digest, if not a new digest is created for the email. - * - * @param \Cake\ORM\Entity $emailQueueEntity An email entity - * @return \Passbolt\EmailDigest\Utility\Digest\DigestInterface - * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException + * @return \App\Model\Entity\User */ - public function addEmailEntity(Entity $emailQueueEntity): DigestInterface + public function getOperator(): User { - if (!$this->canAddToDigest($emailQueueEntity)) { - throw new UnsupportedEmailDigestDataException($emailQueueEntity); - } - - $operator = $this->getOperatorFromEmail($emailQueueEntity); - if (!isset($operator['username']) || !EmailValidationRule::check($operator['username'])) { - throw new UnsupportedEmailDigestDataException($emailQueueEntity); - } - - $operator = $operator['username']; - $username = $emailQueueEntity['email']; - $this->addToUserCount($username, $operator); - - $this->emails[$username][$operator][] = $emailQueueEntity; - - return $this; + return $this->operator; } /** - * Process and set the content of the emails (as EmailDigest). - * - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] + * @return bool */ - public function marshalEmails(): array + public function isRecipientTheOperator(): bool { - $result = []; - foreach ($this->emails as $username => $emailsByOperator) { - foreach ($emailsByOperator as $operator => $emails) { - $numberOfEmails = $this->getEmailCount($username, $operator); - $renderFromEmailPreview = true; - if ($numberOfEmails === 1) { - $digest = $this->buildSingleEmailDigest($emails[0]); - } elseif (!$this->isPassThreshold($username, $operator)) { - $digest = $this->buildMultipleEmailDigest($emails); - } else { - $digest = $this->onThresholdCallback($emails, $numberOfEmails); - $renderFromEmailPreview = false; // Do not render the emails in the digest - } - if ($renderFromEmailPreview) { - $digest->setContent( - $this->renderDigestContentFromEmailPreview($this->emailPreviewFactory, $digest) - ); - } - - $result[] = $digest; - } - } - - /** @var \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] $result */ - return $result; + return $this->getOperator()->username === $this->recipient; } /** - * @param array $emails array of EmailQueue Entity - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigest + * @return \Passbolt\EmailDigest\Utility\Digest\AbstractDigestTemplate */ - protected function buildMultipleEmailDigest(array $emails): EmailDigest + public function getTemplate(): AbstractDigestTemplate { - $digest = new EmailDigest(); - foreach ($emails as $i => $emailQueueEntity) { - $operator = $this->getOperatorFromEmail($emailQueueEntity); - if (!isset($operator['profile']['first_name']) || !is_string($operator['profile']['first_name'])) { - throw new UnsupportedEmailDigestDataException($emailQueueEntity); - } - $subject = $this->getTranslatedSubject($operator['profile']['first_name'], $emailQueueEntity); - $digest - ->addEmailData($emailQueueEntity) - ->setSubject($subject) - ->setEmailRecipient($emailQueueEntity->email); - } - - return $digest; + return $this->template; } /** - * Single email digest can always add any email. + * The recipient of the digest * - * @param \Cake\ORM\Entity $emailQueueEntity An email entity - * @return bool + * @return string */ - public function canAddToDigest(Entity $emailQueueEntity): bool + public function getRecipient(): string { - $executedBy = $this->getOperatorFromEmail($emailQueueEntity); - - return !empty($executedBy) && $this->isTemplateSupported($emailQueueEntity->get('template')); + return $this->recipient; } /** - * Return true if the template is supported by the digest, false otherwise. + * The full base URL covered by the digest * - * @param string $template Template to use - * @return bool + * @return string */ - private function isTemplateSupported(string $template): bool + public function getFullBaseUrl(): string { - return empty($this->supportedTemplates) || in_array($template, $this->supportedTemplates); + return $this->fullBaseUrl; } /** - * Return the user from the variable of the email. - * - * @param \Cake\ORM\Entity $emailData An email queue entity - * @return \App\Model\Entity\User|null + * @return \Cake\ORM\Entity[] */ - private function getOperatorFromEmail(Entity $emailData) + public function getEmailQueues(): array { - return $emailData->get('template_vars')['body'][$this->executedByTemplateVarKey] ?? null; + return $this->emailQueues; } /** - * Add +1 to the total email count for a given user - * - * @param string $username email of the user affected by the action - * @param string $operator email of the user doing the action - * @return void + * @return \Cake\ORM\Entity */ - private function addToUserCount(string $username, string $operator) + public function getFirstEmailQueue(): Entity { - if (!isset($this->emailCount[$username][$operator])) { - $this->emailCount[$username][$operator] = 1; - } else { - $this->emailCount[$username][$operator]++; - } + return $this->emailQueues[0]; } /** - * Return email count for given user and operator + * Get a list of all the email ids * - * @param string $username email of the user affected by the action - * @param string $operator email of the user doing the action - * @return int + * @return array */ - private function getEmailCount(string $username, string $operator) + public function getEmailQueueIds(): array { - if (!isset($this->emailCount[$username]) || !isset($this->emailCount[$username][$operator])) { - return 0; - } - - return $this->emailCount[$username][$operator]; + return (array)Hash::extract($this->emailQueues, '{n}.id'); } /** - * Return true if threshold is passed for a given user and operator + * Get the number of emails in the digest * - * @param string $username email of the user affected by the action - * @param string $operator email of the user doing the action - * @return bool + * @return int */ - private function isPassThreshold(string $username, string $operator) + public function getEmailQueueCount(): int { - return $this->emailCount[$username][$operator] > self::MAXIMUM_EMAILS_IN_DIGEST; + return count($this->emailQueues); } /** - * Callback executed when the maximum threshold defined is reached. - * - * Must return an array of email digests. + * Append an email to this digest * - * @param \Cake\ORM\Entity[] $emailQueueEntities An email queue entities - * @param int $emailCount Count of the emails - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] + * @param \Cake\ORM\Entity $emailQueue Email queue + * @return $this */ - private function onThresholdCallback(array $emailQueueEntities, int $emailCount) + public function addEmail(Entity $emailQueue) { - return call_user_func($this->onThresholdCallback, $emailQueueEntities, $emailCount); + $this->emailQueues[] = $emailQueue; + + return $this; } /** - * @param string $operatorFirstName First name of the operator - * @param \Cake\ORM\Entity $emailQueueEntity EmailQueue Entity - * @return string + * Render the digest into an Email Digest ready to be sent + * + * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface */ - private function getTranslatedSubject(string $operatorFirstName, Entity $emailQueueEntity): string + public function marshalEmails(): EmailDigestInterface { - $emailLocale = $emailQueueEntity['template_vars'][LocaleEmailQueueListener::VIEW_VAR_KEY] ?? null; + $numberOfEmails = $this->getEmailQueueCount(); + $renderFromEmailPreview = true; + $emailPreviewFactory = new EmailPreviewFactory(); + if ($numberOfEmails === 1) { + // If the digest contains only one email, render one single email + $emailDigest = $emailPreviewFactory->buildSingleEmailDigest($this->getFirstEmailQueue()); + } elseif ($numberOfEmails <= self::MAXIMUM_EMAILS_IN_DIGEST) { + // If the digest contains between 2 and 10 emails, render all the emails in one single digest + $emailDigest = $emailPreviewFactory->buildMultipleEmailDigest($this); + } else { + // If the digest contains more than 10 emails, render only one single email summarizing what happened + $emailDigest = $emailPreviewFactory->buildSummaryEmailDigest($this); + $renderFromEmailPreview = false; // Do not render the emails in the digest + } + if ($renderFromEmailPreview) { + $emailDigest->setContent( + $emailPreviewFactory->renderDigestContentFromEmailPreview($emailDigest) + ); + } - $makeSubject = function () use ($operatorFirstName): string { - return __($this->subject, $operatorFirstName); - }; + return $emailDigest; + } - if ($emailLocale !== null) { - $subject = (new LocaleService())->translateString( - $emailLocale, - $makeSubject - ); - } else { - $subject = $makeSubject(); + /** + * Get the locale of the first email, which is the same for all emails + * as the recipient is the same for all emails of a digest + * + * @return string|null + */ + public function getLocale(): ?string + { + $locale = $this->getFirstEmailQueue()->get('template_vars')['locale'] ?? null; + if (is_null($locale)) { + Log::error('No locale was defined for this email.'); } - return $subject; + return $locale; } } diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestRegisterEvent.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestRegisterEvent.php deleted file mode 100644 index c68b60f4ac..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestRegisterEvent.php +++ /dev/null @@ -1,62 +0,0 @@ -getSubject(); - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestTemplateRegistry.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestTemplateRegistry.php new file mode 100644 index 0000000000..42a685132a --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestTemplateRegistry.php @@ -0,0 +1,98 @@ +hasTemplate($className)) { + return $this; + } + $this->digestTemplates[$className] = $template; + + return $this; + } + + /** + * @param string $templateClassName template FQN + * @return bool + */ + public function hasTemplate(string $templateClassName): bool + { + return array_key_exists($templateClassName, $this->digestTemplates); + } + + /** + * @return \Passbolt\EmailDigest\Utility\Digest\AbstractDigestTemplate[] + */ + public function getTemplates(): array + { + return $this->digestTemplates; + } +} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsCollection.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsCollection.php deleted file mode 100644 index 902ee70dd8..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsCollection.php +++ /dev/null @@ -1,98 +0,0 @@ -digestsPool = $digestsPool ?? DigestsPool::getInstance(); - } - - /** - * Foreach each email entity, it goes through each email digests, - * check if it can digest the email entity, if yes add emails data to it. - * The first digest to be picked in the list, if can be used, will be the first and ONLY one digest served for the email. - * - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] - */ - public function marshalEmails(): array - { - $digests = []; - - foreach ($this->digestsPool->getDigests() as $digest) { - $digests = array_merge($digests, $digest->marshalEmails()); - } - - return $digests; - } - - /** - * A digest collection can work with an email entity if at least one of the digests - * in the collection can work with the email - * - * @param \Cake\ORM\Entity $emailQueueEntity An email entity from email queue - * @return bool return false if no digest in the pool supports the email data. - */ - public function canAddToDigest(Entity $emailQueueEntity): bool - { - foreach ($this->digestsPool->getDigests() as $digest) { - if ($digest->canAddToDigest($emailQueueEntity)) { - return true; - } - } - - return false; - } - - /** - * @param \Cake\ORM\Entity $emailQueueEntity An instance of email entity - * @return \Passbolt\EmailDigest\Utility\Digest\DigestInterface - * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException - */ - public function addEmailEntity(Entity $emailQueueEntity): DigestInterface - { - foreach ($this->digestsPool->getDigests() as $digest) { - if ($digest->canAddToDigest($emailQueueEntity)) { - $digest->addEmailEntity($emailQueueEntity); - - return $this; - } - } - - throw new UnsupportedEmailDigestDataException($emailQueueEntity); - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsPool.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsPool.php deleted file mode 100644 index 61dc73fc42..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestsPool.php +++ /dev/null @@ -1,117 +0,0 @@ -digests[] = [ - 'digest' => $digest, - 'priority' => $priority, - ]; - - return $this; - } - - /** - * Return a collection of email digests ordered by priority. - * - * @return \Passbolt\EmailDigest\Utility\Digest\DigestInterface[] - */ - public function getDigests(): array - { - $digests = $this->digests; - - // Sort the digests by priority in ascendant order. - usort($digests, function ($digestA, $digestB) { - if ($digestA['priority'] == $digestB['priority']) { - return 0; - } - // -1 if priority of digest A is lower than the priority of digest B - // 0 if priority of digest A is equal to priority of digest B - // 1 if priority of digest A is greater than the priority of digest B - return $digestA['priority'] < $digestB['priority'] ? -1 : 1; - }); - - // Then we reverse the array to make it in descendant order - // since we want to start with the digest with the highest priority, not the lowest. - $digests = array_reverse($digests); - - return array_column($digests, 'digest'); - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/SingleDigest.php b/plugins/PassboltCe/EmailDigest/src/Utility/Digest/SingleDigest.php deleted file mode 100644 index d789dbb025..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/SingleDigest.php +++ /dev/null @@ -1,89 +0,0 @@ -emailPreviewFactory = $emailPreviewFactory ?? new EmailPreviewFactory(); - } - - /** - * Add an email - * - * @param \Cake\ORM\Entity $emailQueueEntity An email entity - * @return \Passbolt\EmailDigest\Utility\Digest\DigestInterface - */ - public function addEmailEntity(Entity $emailQueueEntity): DigestInterface - { - $this->emails[] = $emailQueueEntity; - - return $this; - } - - /** - * Process and set the content of the emails (as EmailDigest). - * - * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] - */ - public function marshalEmails(): array - { - $result = []; - foreach ($this->emails as $username => $emails) { - $result[] = $this->buildSingleEmailDigest($emails); - } - foreach ($result as $digest) { - $digest->setContent($this->renderDigestContentFromEmailPreview($this->emailPreviewFactory, $digest)); - } - - return $result; - } - - /** - * Single email digest can always add any email. - * - * @param \Cake\ORM\Entity $emailQueueEntity An email entity - * @return bool - */ - public function canAddToDigest(Entity $emailQueueEntity): bool - { - return true; - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestInterface.php b/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/AbstractDigestCollection.php similarity index 68% rename from plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestInterface.php rename to plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/AbstractDigestCollection.php index 8798cbf518..7ed17fa1c7 100644 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Digest/DigestInterface.php +++ b/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/AbstractDigestCollection.php @@ -12,10 +12,10 @@ * @copyright Copyright (c) Passbolt SA (https://www.passbolt.com) * @license https://opensource.org/licenses/AGPL-3.0 AGPL License * @link https://www.passbolt.com Passbolt(tm) - * @since 2.13.0 + * @since 4.5.0 */ -namespace Passbolt\EmailDigest\Utility\Digest; +namespace Passbolt\EmailDigest\Utility\DigestCollection; use Cake\ORM\Entity; @@ -30,15 +30,15 @@ * * @see EmailDigestInterface */ -interface DigestInterface +abstract class AbstractDigestCollection { /** - * Add some email entities to digest. + * Add some email entities to digest collection * - * @param \Cake\ORM\Entity $emailQueueEntity An email entity to add to the digest + * @param \Cake\ORM\Entity $emailQueue An email entity to add to the digest collection * @return self */ - public function addEmailEntity(Entity $emailQueueEntity): DigestInterface; + abstract public function addEmailEntity(Entity $emailQueue): self; /** * Return a list of emails. Even if the digest return one, the digest must be in an array. @@ -46,13 +46,5 @@ public function addEmailEntity(Entity $emailQueueEntity): DigestInterface; * * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] */ - public function marshalEmails(): array; - - /** - * Return a boolean indicating if the digest can handle the given email entity from email queue and marshal it into an email digest. - * - * @param \Cake\ORM\Entity $emailQueueEntity An instance of EmailDigest - * @return bool - */ - public function canAddToDigest(Entity $emailQueueEntity): bool; + abstract public function marshalEmails(): array; } diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/DigestsCollection.php b/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/DigestsCollection.php new file mode 100644 index 0000000000..b160d5a143 --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/DigestsCollection.php @@ -0,0 +1,184 @@ +getSortedDigests() as $digest) { + $emailDigests[] = $digest->marshalEmails(); + } + + return $emailDigests; + } + + /** + * @return array + */ + private function getSortedDigests(): array + { + $digests = $this->digests; + + // Sort the digests by priority in ascendant order. + usort($digests, function (Digest $digestA, Digest $digestB) { + $priorityA = $digestA->getTemplate()->getPriority(); + $priorityB = $digestB->getTemplate()->getPriority(); + + if ($priorityA == $priorityB) { + return 0; + } + // -1 if priority of digest A is lower than the priority of digest B + // 0 if priority of digest A is equal to priority of digest B + // 1 if priority of digest A is greater than the priority of digest B + return $priorityA < $priorityB ? -1 : 1; + }); + + return $digests; + } + + /** + * Append the email queue to an exiting digest + * or create a new digest if no digests exist for this operator + * and template. + * + * If the email queue cannot be added to any template, an exception is thrown, + * which should be caught to create a SingleDigest for this email queue. + * + * @param \Cake\ORM\Entity $emailQueue An instance of email entity + * @return self + * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException + */ + public function addEmailEntity(Entity $emailQueue): self + { + $digestTemplate = $this->getDigestTemplateFromEmailQueueOrFail($emailQueue); + $operator = $this->getOperatorFromEmailQueue($digestTemplate, $emailQueue); + $recipient = $emailQueue->get('email'); + $fullBaseUrl = $this->getFullBaseUrlFromEmailQueue($emailQueue); + // Check if a digest already exists for this email queue + foreach ($this->digests as $digest) { + $isSameRecipient = $digest->getRecipient() === $recipient; + $isSameOperator = $digest->getOperator()->username === $operator->username; + $isSameFullBaseUrl = $digest->getFullBaseUrl() === $fullBaseUrl; + $isSameDigestTemplate = $digest->getTemplate() instanceof $digestTemplate; + + if ($isSameRecipient && $isSameOperator && $isSameFullBaseUrl && $isSameDigestTemplate) { + $digest->addEmail($emailQueue); + + return $this; + } + } + + // Otherwise, create a new digest and add this email queue to it + $this->digests[] = (new Digest($recipient, $operator, $fullBaseUrl, $emailQueue, $digestTemplate)); + + return $this; + } + + /** + * Return the user from the variable of the email. + * + * @param \Passbolt\EmailDigest\Utility\Digest\AbstractDigestTemplate $digestTemplate digest template for this email queue + * @param \Cake\ORM\Entity $emailQueue An email queue entity + * @return \App\Model\Entity\User + * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException if the operator is not set or invalid + */ + private function getOperatorFromEmailQueue(AbstractDigestTemplate $digestTemplate, Entity $emailQueue): User + { + $operator = $emailQueue->get('template_vars')['body'][$digestTemplate->getOperatorVariableKey()] ?? []; + if (!isset($operator['profile']['first_name']) || !is_string($operator['profile']['first_name'])) { + throw new UnsupportedEmailDigestDataException($emailQueue); + } + if (!isset($operator['profile']['last_name']) || !is_string($operator['profile']['last_name'])) { + throw new UnsupportedEmailDigestDataException($emailQueue); + } + if (!isset($operator['username']) || !EmailValidationRule::check($operator['username'])) { + throw new UnsupportedEmailDigestDataException($emailQueue); + } + + return new User($operator); + } + + /** + * Return the full base url from the template vars of the email. + * + * @param \Cake\ORM\Entity $emailQueue An email queue entity + * @return string + * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException if the full base url is not defined or empty + */ + private function getFullBaseUrlFromEmailQueue(Entity $emailQueue): string + { + $fullBaseUrl = $emailQueue->get('template_vars')['body']['fullBaseUrl'] ?? null; + if (empty($fullBaseUrl) || !is_string($fullBaseUrl)) { + throw new UnsupportedEmailDigestDataException($emailQueue); + } + + return $fullBaseUrl; + } + + /** + * A digest collection can integrate an email entity into one of its digests + * if at least one of the digests in the template registry accepts this email + * + * @param \Cake\ORM\Entity $emailQueue An email entity from email queue + * @return \Passbolt\EmailDigest\Utility\Digest\AbstractDigestTemplate return false if no digest in the pool supports the email data + * @throws \Passbolt\EmailDigest\Exception\UnsupportedEmailDigestDataException if no digest template was found for this email queue + */ + private function getDigestTemplateFromEmailQueueOrFail(Entity $emailQueue): AbstractDigestTemplate + { + // All the digest templates in the pool + $allDigestTemplates = DigestTemplateRegistry::getInstance()->getTemplates(); + // Template of the email queue + $emailTemplate = $emailQueue->get('template'); + foreach ($allDigestTemplates as $template) { + if (in_array($emailTemplate, $template->getSupportedTemplates())) { + return $template; + } + } + + throw new UnsupportedEmailDigestDataException($emailQueue); + } +} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/SingleDigestCollection.php b/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/SingleDigestCollection.php new file mode 100644 index 0000000000..8ad595ee14 --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/src/Utility/DigestCollection/SingleDigestCollection.php @@ -0,0 +1,63 @@ +emails[] = $emailQueue; + + return $this; + } + + /** + * Process and set the content of the emails (as EmailDigest). + * + * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface[] + */ + public function marshalEmails(): array + { + $emailPreviewFactory = new EmailPreviewFactory(); + $result = []; + foreach ($this->emails as $email) { + $emailDigest = $emailPreviewFactory->buildSingleEmailDigest($email); + $emailDigest->setContent($emailPreviewFactory->renderDigestContentFromEmailPreview($emailDigest)); + $result[] = $emailDigest; + } + + return $result; + } +} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Factory/DigestFactory.php b/plugins/PassboltCe/EmailDigest/src/Utility/Factory/DigestFactory.php deleted file mode 100644 index c07296a22e..0000000000 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Factory/DigestFactory.php +++ /dev/null @@ -1,109 +0,0 @@ -isDigestRegisterEventDispatched = false; - $this->digestsPool = $digestsPool ?? DigestsPool::getInstance(); - } - - /** - * Return a singleton of the DigestsPool - * - * @return static - */ - public static function getInstance(): self - { - if (!isset(static::$instance)) { - static::$instance = new static(); - } - - return static::$instance; - } - - /** - * Clear the singleton of the DigestFactory - * - * @return void - */ - public static function clearInstance(): void - { - static::$instance = null; - } - - /** - * Factory method for DigestsCollection - * - * @return \Passbolt\EmailDigest\Utility\Digest\SingleDigest - */ - public function createSingleDigest(): SingleDigest - { - return new SingleDigest(); - } - - /** - * Factory method for DigestsCollection - * - * @return \Passbolt\EmailDigest\Utility\Digest\DigestsCollection - */ - public function createDigestsCollection(): DigestsCollection - { - if (!$this->isDigestRegisterEventDispatched) { - // Dispatch an event to offer possibility to other components to register more digests. - // Event must be dispatched only once to avoid unnecessary extra registration - EventManager::instance()->dispatch(DigestRegisterEvent::create($this->digestsPool)); - $this->isDigestRegisterEventDispatched = true; - } - - return new DigestsCollection($this->digestsPool); - } -} diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Factory/EmailPreviewFactory.php b/plugins/PassboltCe/EmailDigest/src/Utility/Factory/EmailPreviewFactory.php index c1bb066d4d..9bcc634f41 100644 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Factory/EmailPreviewFactory.php +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Factory/EmailPreviewFactory.php @@ -17,11 +17,15 @@ namespace Passbolt\EmailDigest\Utility\Factory; +use Cake\Core\Configure; use Cake\Mailer\Mailer; use Cake\Mailer\Renderer; use Cake\ORM\Entity; +use Passbolt\EmailDigest\Utility\Digest\Digest; +use Passbolt\EmailDigest\Utility\Mailer\EmailDigest; use Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface; use Passbolt\EmailDigest\Utility\Mailer\EmailPreview; +use Passbolt\Locale\Event\LocaleEmailQueueListener; /** * Create email previews from an email entity or from an email digest. @@ -62,7 +66,7 @@ public function renderEmailPreviewFromDigest( * @param \Cake\ORM\Entity $email Email entity. * @return string */ - public function renderFromEmailEntity(Entity $email): string + private function renderFromEmailEntity(Entity $email): string { $viewVars = empty($email->template_vars) ? [] : $email->template_vars; $viewVars['locale'] = $email->get('template_vars')['locale']; @@ -77,6 +81,80 @@ public function renderFromEmailEntity(Entity $email): string return $renderer->render('', ['html'])['html']; } + /** + * @param \Cake\ORM\Entity $emailQueueEntity entity + * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigest + */ + public function buildSingleEmailDigest(Entity $emailQueueEntity): EmailDigest + { + return (new EmailDigest()) + ->addEmailData($emailQueueEntity) + ->setSubject($emailQueueEntity->get('subject')) + ->setEmailRecipient($emailQueueEntity->get('email')); + } + + /** + * Renders an email digest with multiple emails rendered in it + * + * @param \Passbolt\EmailDigest\Utility\Digest\Digest $digest digest to render + * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigest + */ + public function buildMultipleEmailDigest(Digest $digest): EmailDigest + { + $subject = $digest->getTemplate()->getTranslatedSubject($digest); + + $emailDigest = new EmailDigest(); + foreach ($digest->getEmailQueues() as $emailQueueEntity) { + $emailDigest + ->addEmailData($emailQueueEntity) + ->setSubject($subject) + ->setEmailRecipient($emailQueueEntity->get('email')); + } + + return $emailDigest; + } + + /** + * Renders an email with a summary of what happened in the 11+ emails of the digest + * + * @param \Passbolt\EmailDigest\Utility\Digest\Digest $digest digest + * @return \Passbolt\EmailDigest\Utility\Mailer\EmailDigest + */ + public function buildSummaryEmailDigest(Digest $digest): EmailDigest + { + $template = $digest->getTemplate(); + $subject = $template->getTranslatedSubject($digest); + + return (new EmailDigest()) + ->addEmailData($digest->getFirstEmailQueue()) + ->setEmailIds($digest->getEmailQueueIds()) + ->setSubject($subject) + ->addLayoutVar(LocaleEmailQueueListener::VIEW_VAR_KEY, $digest->getLocale()) + ->setTemplate($template->getDigestTemplate()) + ->setEmailRecipient($digest->getRecipient()) + ->addTemplateVar($template->getOperatorVariableKey(), $digest->getOperator()) + ->addTemplateVar('fullBaseUrl', Configure::read('App.fullBaseUrl')) + ->addTemplateVar('subject', $subject) + ->addTemplateVar('count', $digest->getEmailQueueCount()); + } + + /** + * Helper method which render the content of every emails contained in a digest into a string to be used + * as the content of the digest. + * + * @param \Passbolt\EmailDigest\Utility\Mailer\EmailDigestInterface $emailDigest Email digest to use to render + * @return string + */ + public function renderDigestContentFromEmailPreview(EmailDigestInterface $emailDigest): string + { + $emailDigestContent = []; + foreach ($emailDigest->getEmailsData() as $emailData) { + $emailDigestContent[] = $this->renderFromEmailEntity($emailData); + } + + return implode('', $emailDigestContent); + } + /** * Return a preview of the email as it was sent with its headers and its content. * The Email::send() method handle the render of the email. It call the send method of its configured diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigest.php b/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigest.php index 1e226e4af4..5a3d38ae90 100644 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigest.php +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigest.php @@ -18,7 +18,6 @@ namespace Passbolt\EmailDigest\Utility\Mailer; use Cake\ORM\Entity; -use Cake\Utility\Hash; class EmailDigest implements EmailDigestInterface { @@ -37,56 +36,42 @@ class EmailDigest implements EmailDigestInterface */ public const TPL_VAR_DIGEST_CONTENT = 'digest_content'; - /** - * @var array - */ - private $emailsData = []; - - /** - * @var string - */ - private $content; - - /** - * @var string - */ - private $recipient; - - /** - * @var string - */ - private $subject; - + private array $emailsData = []; + private array $emailIds = []; + private string $content; + private string $recipient; + private string $subject; /** * @var string|null Template to use to compose the email */ - private $template = null; - - /** - * @var array - */ - private $templateVars = []; - - /** - * @var array - */ - private $layoutVars = []; + private ?string $template = null; + private array $templateVars = []; + private array $layoutVars = []; /** * Return the list of ids of the emails part of the digest * * @return string[] */ - public function getEmailIds() + public function getEmailIds(): array { - return Hash::extract($this->emailsData, '{n}.id'); + return $this->emailIds; } /** - * @param \Cake\ORM\Entity $emailQueueEntity An instance of EmailQueue entity - * @return $this + * @inheritDoc */ - public function addEmailData(Entity $emailQueueEntity) + public function setEmailIds(array $emailIds): self + { + $this->emailIds = $emailIds; + + return $this; + } + + /** + * @inheritDoc + */ + public function addEmailData(Entity $emailQueueEntity): self { $this->emailsData[] = $emailQueueEntity; @@ -94,9 +79,9 @@ public function addEmailData(Entity $emailQueueEntity) } /** - * @return \Cake\ORM\Entity[] + * @inheritDoc */ - public function getEmailsData() + public function getEmailsData(): array { return $this->emailsData; } @@ -106,7 +91,7 @@ public function getEmailsData() * * @return array */ - public function getViewVars() + public function getViewVars(): array { $vars = $this->templateVars; @@ -120,19 +105,17 @@ public function getViewVars() } /** - * @return string + * @inheritDoc */ - public function getEmailFormat() + public function getEmailFormat(): string { return self::DEFAULT_FORMAT; } /** - * Return the path of the template file to use for this email - * - * @return string + * @inheritDoc */ - public function getTemplate() + public function getTemplate(): string { return $this->template ?? self::DEFAULT_TEMPLATE; } @@ -140,18 +123,15 @@ public function getTemplate() /** * @return string */ - public function getContent() + public function getContent(): string { return $this->content; } /** - * Return the email recipient - * - * @param string $recipient Email Recipient of the digest, i.e: ada@passbolt.com - * @return $this + * @inheritDoc */ - public function setEmailRecipient(string $recipient) + public function setEmailRecipient(string $recipient): self { $this->recipient = $recipient; @@ -159,20 +139,17 @@ public function setEmailRecipient(string $recipient) } /** - * Return the email recipient - * - * @return string + * @inheritDoc */ - public function getEmailRecipient() + public function getEmailRecipient(): string { return $this->recipient; } /** - * @param string $subject Subject of the digest - * @return $this + * @inheritDoc */ - public function setSubject(string $subject) + public function setSubject(string $subject): self { $this->subject = $subject; @@ -184,18 +161,15 @@ public function setSubject(string $subject) * * @return string */ - public function getSubject() + public function getSubject(): string { return $this->subject; } /** - * Define the content of the email digest - * - * @param string $digestContent Content of the digest - * @return $this + * @inheritDoc */ - public function setContent(string $digestContent) + public function setContent(string $digestContent): self { $this->content = $digestContent; @@ -203,10 +177,9 @@ public function setContent(string $digestContent) } /** - * @param string $template Template - * @return $this + * @inheritDoc */ - public function setTemplate(string $template) + public function setTemplate(string $template): self { $this->template = $template; @@ -214,13 +187,9 @@ public function setTemplate(string $template) } /** - * Add key/value pair to "body" variable available in email view - * - * @param string $name Name of the variable to add to be used with the template of the email - * @param mixed $value Value of the variable - * @return $this + * @inheritDoc */ - public function addTemplateVar(string $name, $value) + public function addTemplateVar(string $name, $value): self { $this->templateVars[$name] = $value; @@ -228,13 +197,9 @@ public function addTemplateVar(string $name, $value) } /** - * Add variable available in email template - * - * @param string $name Name of the variable to add to be used with the layout of the email - * @param mixed $value Value of the variable - * @return $this + * @inheritDoc */ - public function addLayoutVar(string $name, $value) + public function addLayoutVar(string $name, $value): self { $this->layoutVars[$name] = $value; diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigestInterface.php b/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigestInterface.php index d5a992cdc4..88e7a09277 100644 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigestInterface.php +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailDigestInterface.php @@ -28,23 +28,23 @@ interface EmailDigestInterface extends EmailInterface * An array of data composed with email queue format * * @param \Cake\ORM\Entity $emailQueueEntity An email entity - * @return $this + * @return self */ - public function addEmailData(Entity $emailQueueEntity); + public function addEmailData(Entity $emailQueueEntity): self; /** * Return the emails entity associated to the email digest * * @return \Cake\ORM\Entity[] */ - public function getEmailsData(); + public function getEmailsData(): array; /** * Render the content of the email digest. * * @return string */ - public function getContent(); + public function getContent(): string; /** * A digest must be able to return the ids of the emails which compose the digest. @@ -52,17 +52,19 @@ public function getContent(); * * @return string[] */ - public function getEmailIds(); + public function getEmailIds(): array; /** + * Define the content of the email digest + * * @param string $digestContent Content of the digest - * @return $this + * @return self */ - public function setContent(string $digestContent); + public function setContent(string $digestContent): self; /** * @param string $template Template - * @return $this + * @return self */ - public function setTemplate(string $template); + public function setTemplate(string $template): self; } diff --git a/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailInterface.php b/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailInterface.php index 83c18198c1..78bcafa6c3 100644 --- a/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailInterface.php +++ b/plugins/PassboltCe/EmailDigest/src/Utility/Mailer/EmailInterface.php @@ -28,7 +28,7 @@ interface EmailInterface * * @return string */ - public function getEmailRecipient(); + public function getEmailRecipient(): string; /** * Return the subject of the email digest @@ -36,14 +36,14 @@ public function getEmailRecipient(); * * @return string */ - public function getSubject(); + public function getSubject(): string; /** * Return the template vars associated to the digest * * @return array */ - public function getViewVars(); + public function getViewVars(): array; /** * Return the format of the email digest @@ -51,7 +51,7 @@ public function getViewVars(); * * @return string */ - public function getEmailFormat(); + public function getEmailFormat(): string; /** * Return the template file to use with this email digest @@ -59,35 +59,45 @@ public function getEmailFormat(); * * @return string */ - public function getTemplate(); + public function getTemplate(): string; /** * Add a variable to the template variables for the email. * * @param string $name Name of the variable * @param mixed $value Value of the variable - * @return $this + * @return self */ - public function addTemplateVar(string $name, $value); + public function addTemplateVar(string $name, $value): self; /** * Add a variable to the layout variables for the email. * * @param string $name Name of the variable * @param mixed $value Value of the variable - * @return $this + * @return self */ - public function addLayoutVar(string $name, $value); + public function addLayoutVar(string $name, $value): self; /** * @param string $subject Subject of the digest - * @return $this + * @return self */ - public function setSubject(string $subject); + public function setSubject(string $subject): self; /** - * @param string $recipient Recipient of the digest - * @return $this + * Return the email recipient + * + * @param string $recipient Email Recipient of the digest, i.e: ada@passbolt.com + * @return self + */ + public function setEmailRecipient(string $recipient): self; + + /** + * Sets the list of ids of the emails part of the digest + * + * @param string[] $emailIds email ids of the emails under this email digest + * @return self */ - public function setEmailRecipient(string $recipient); + public function setEmailIds(array $emailIds): self; } diff --git a/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php b/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php index 9e39c1eb9e..9df70921d8 100644 --- a/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php +++ b/plugins/PassboltCe/EmailDigest/tests/Factory/EmailQueueFactory.php @@ -11,8 +11,9 @@ /** * EmailQueueFactory * - * @method \Cake\Datasource\EntityInterface persist() - * @method \Cake\Datasource\EntityInterface getEntity() + * @method \Cake\ORM\Entity|\Cake\ORM\Entity[] persist() + * @method \Cake\ORM\Entity getEntity() + * @method \Cake\ORM\Entity[] getEntities() */ class EmailQueueFactory extends CakephpBaseFactory { @@ -49,6 +50,8 @@ protected function setDefaultTemplate(): void $email = $faker->email(); $title = $faker->sentence(); $locale = 'en-UK'; + $fullBaseUrl = '/'; + $body = compact('fullBaseUrl'); return [ 'email' => $email, @@ -56,7 +59,7 @@ protected function setDefaultTemplate(): void 'config' => 'default', 'template' => 'test_email', 'layout' => 'default', - 'template_vars' => json_encode(compact('email', 'title', 'locale')), + 'template_vars' => json_encode(compact('email', 'title', 'locale', 'fullBaseUrl', 'body')), 'theme' => '', 'format' => 'html', 'sent' => 0, @@ -132,4 +135,33 @@ public function sent() { return $this->setField('sent', true); } + + /** + * @param string $fullBaseUrl full base url + * @return EmailQueueFactory + */ + public function setFullBaseUrl(string $fullBaseUrl) + { + return $this->setField('template_vars.body.fullBaseUrl', $fullBaseUrl); + } + + /** + * @param string $subject locale of the email + * @return EmailQueueFactory + */ + public function setSubject(string $subject) + { + $this->setField('template_vars.body.subject', $subject); + + return $this->setField('subject', $subject); + } + + /** + * @param string $locale locale of the email + * @return EmailQueueFactory + */ + public function setLocale(string $locale) + { + return $this->setField('template_vars.locale', $locale); + } } diff --git a/plugins/PassboltCe/EmailDigest/tests/Factory/GroupUserDeleteEmailQueueFactory.php b/plugins/PassboltCe/EmailDigest/tests/Factory/GroupUserDeleteEmailQueueFactory.php new file mode 100644 index 0000000000..c9d7a0c02c --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/tests/Factory/GroupUserDeleteEmailQueueFactory.php @@ -0,0 +1,56 @@ +setTemplate(GroupUserDeleteEmailRedactor::TEMPLATE); + $this->setGroup(); + $this->setOperator(); + } + + /** + * @param User|null $user user who deleted the resource + * @return self + */ + public function setOperator(?User $user = null) + { + if (is_null($user)) { + $user = UserFactory::make()->getEntity(); + } + + return $this->setField('template_vars.body.admin', $user->toArray()); + } + + /** + * @param Group|null $group resource deleted + * @return self + */ + public function setGroup(?Group $group = null) + { + if (is_null($group)) { + $group = GroupFactory::make()->getEntity(); + } + + return $this->setField('template_vars.body.group', $group->toArray()); + } +} diff --git a/plugins/PassboltCe/EmailDigest/tests/Factory/ResourceDeleteEmailQueueFactory.php b/plugins/PassboltCe/EmailDigest/tests/Factory/ResourceDeleteEmailQueueFactory.php new file mode 100644 index 0000000000..36ae6b315b --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/tests/Factory/ResourceDeleteEmailQueueFactory.php @@ -0,0 +1,65 @@ +setTemplate(ResourceDeleteEmailRedactor::TEMPLATE) + ->setResource() + ->setOperator() + ->setSubject($this->getFaker()->sentence()) + ->setField('template_vars.body.showUsername', true) + ->setField('template_vars.body.showUri', true) + ->setField('template_vars.body.showDescription', true); + } + + /** + * @param User|null $user user who deleted the resource + * @return ResourceDeleteEmailQueueFactory + */ + public function setOperator(?User $user = null) + { + if (is_null($user)) { + $user = UserFactory::make()->getEntity(); + } + + return $this->setField('template_vars.body.user', $user->toArray()); + } + + /** + * @param Resource|null $resource resource deleted + * @return ResourceDeleteEmailQueueFactory + */ + public function setResource(?Resource $resource = null) + { + if (is_null($resource)) { + $resource = ResourceFactory::make()->getEntity(); + } + + return $this->setField('template_vars.body.resource', $resource->toArray()); + } +} diff --git a/plugins/PassboltCe/EmailDigest/tests/Lib/EmailDigestMockTestTrait.php b/plugins/PassboltCe/EmailDigest/tests/Lib/EmailDigestMockTestTrait.php index d9ed8fd482..5b0169c749 100644 --- a/plugins/PassboltCe/EmailDigest/tests/Lib/EmailDigestMockTestTrait.php +++ b/plugins/PassboltCe/EmailDigest/tests/Lib/EmailDigestMockTestTrait.php @@ -16,121 +16,12 @@ */ namespace Passbolt\EmailDigest\Test\Lib; -use App\Model\Entity\Profile; -use App\Model\Entity\User; use App\Test\Factory\ResourceFactory; use App\Test\Factory\UserFactory; -use Cake\ORM\Entity; use Passbolt\EmailDigest\Test\Factory\EmailQueueFactory; -use Passbolt\EmailDigest\Utility\Digest\AbstractDigest; -use Passbolt\EmailDigest\Utility\Digest\DigestInterface; -use Passbolt\EmailDigest\Utility\Mailer\EmailDigest; trait EmailDigestMockTestTrait { - /** - * Create a new digest at runtime for testing. - * - * @param bool $canAddToDigests Returned by canAddToDigests method of the digest. - * @param array $digests Some email digests that the digest must return - * @return \Passbolt\EmailDigest\Utility\Digest\DigestInterface - */ - public function createDigest(bool $canAddToDigests, array $digests): DigestInterface - { - return new class ($canAddToDigests, $digests) extends AbstractDigest - { - private $canAddToDigests; - private $emailDigests = []; - - public function __construct(bool $canAddToDigests, array $emailDigests) - { - $this->canAddToDigests = $canAddToDigests; - $this->emailDigests = $emailDigests; - } - - public function marshalEmails(): array - { - return $this->emailDigests; - } - - public function canAddToDigest(Entity $emailQueueEntity): bool - { - return $this->canAddToDigests; - } - }; - } - - /** - * Create an email queue entity with the passed properties. - * - * @param array|null $properties properties for the email queue entity - * @param array|null $templateBodyVars variables for email body - * @return Entity - */ - protected function createEmailQueueEntity(?array $properties = [], ?array $templateBodyVars = []): Entity - { - $defaultProperties = [ - 'id' => 1, - 'subject' => 'Subject of the email', - 'email' => 'email_queue_recipient@passbolt.com', - 'template' => 'LU/emailQueueTemplate.php', - 'template_vars' => [ - 'body' => array_merge([ - 'owner' => new User(), - 'text' => 'Content of the email', - ], $templateBodyVars), - ], - ]; - - return new Entity(array_merge($defaultProperties, $properties)); - } - - /** - * Create an email digest - * - * @param array $emailsData Array of emails queue entity - * @param array|null $properties props - * @return EmailDigest - */ - protected function createEmailDigest(?array $emailsData = [], ?array $properties = []): EmailDigest - { - $defaultProperties = [ - 'recipient' => 'digest_recipient@passbolt.com', - 'subject' => 'Subject of the email digest', - 'content' => 'Content of the email digest', - ]; - - $properties = array_merge($defaultProperties, $properties); - - $emailDigest = new EmailDigest(); - $emailDigest->setEmailRecipient($properties['recipient']); - $emailDigest->setSubject($properties['subject']); - $emailDigest->setContent($properties['content']); - - foreach ($emailsData as $emailData) { - $emailDigest->addEmailData($emailData); - } - - return $emailDigest; - } - - /** - * Create a User Entity for the email - * - * @param string $username email - * @param string $firstName first name - * @return User - */ - protected function createUserForEmail(string $username, string $firstName) - { - return new User([ - 'username' => $username, - 'profile' => new Profile([ - 'first_name' => $firstName, - ]), - ]); - } - /** * Helper method to save multiple email queue entities for particular user. * Useful to test threshold scenarios. diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/PreviewCommandTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/PreviewCommandTest.php index 3bd42065cc..08ba007c58 100644 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/PreviewCommandTest.php +++ b/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/PreviewCommandTest.php @@ -71,6 +71,7 @@ public function testPreviewCommandHelp(): void */ public function testPreviewCommand_Without_Body(): void { + /** @var array $sender */ $sender = Mailer::getConfig('default')['from']; $senderEmail = array_keys($sender)[0]; $senderName = $sender[$senderEmail]; @@ -117,6 +118,7 @@ public function testPreviewCommandLocale(): void { $this->loadPlugins(['Passbolt/Locale' => []]); $frenchLocale = 'fr-FR'; + /** @var \App\Model\Entity\User $frenchSpeakingUser */ $frenchSpeakingUser = UserFactory::make()->user()->withLocale($frenchLocale)->persist(); EmailQueueFactory::make()->listeningToBeforeSave()->persist(); @@ -143,6 +145,7 @@ public function testPreviewCommand_ThresholdExceeded(): void { (new AvatarsConfigurationService())->loadConfiguration(); $nResourcesAdded = 15; + /** @var \App\Model\Entity\User $operator */ $operator = UserFactory::make()->withAvatar()->persist(); $recipient = 'john@test.test'; EmailQueueFactory::make([ @@ -160,6 +163,6 @@ public function testPreviewCommand_ThresholdExceeded(): void $this->assertMailCount(0); // Make sure preview doesn't send emails $this->assertOutputContains('From: No reply '); $this->assertOutputContains('To: john@test.test'); - $this->assertOutputContains('Subject: Multiple passwords have been changed in passbolt'); + $this->assertOutputContains('Subject: ' . $operator->profile->full_name . ' has made changes on several resources'); } } diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/SenderCommandTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/SenderCommandTest.php index 269db8dc40..ecf3d9548f 100644 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/SenderCommandTest.php +++ b/plugins/PassboltCe/EmailDigest/tests/TestCase/Command/SenderCommandTest.php @@ -16,6 +16,8 @@ */ namespace Passbolt\EmailDigest\Test\TestCase\Command; +use App\Notification\DigestTemplate\GroupMembershipDigestTemplate; +use App\Notification\DigestTemplate\ResourceChangesDigestTemplate; use App\Notification\Email\Redactor\Group\GroupUserAddEmailRedactor; use App\Notification\Email\Redactor\Resource\ResourceCreateEmailRedactor; use App\Service\Avatars\AvatarsConfigurationService; @@ -28,6 +30,7 @@ use Cake\Mailer\Mailer; use Passbolt\EmailDigest\Test\Factory\EmailQueueFactory; use Passbolt\EmailDigest\Test\Lib\EmailDigestMockTestTrait; +use Passbolt\EmailDigest\Utility\Digest\DigestTemplateRegistry; use Passbolt\Locale\Test\Lib\DummyTranslationTestTrait; /** @@ -95,6 +98,7 @@ public function testSenderCommandLocale() $this->loadPlugins(['Passbolt/Locale' => []]); $frenchLocale = 'fr-FR'; + /** @var \App\Model\Entity\User $frenchSpeakingUser */ $frenchSpeakingUser = UserFactory::make()->withLocale($frenchLocale)->persist(); EmailQueueFactory::make(['created' => Chronos::now()->subDays(4)])->persist(); @@ -145,8 +149,8 @@ public function testSenderCommandMultipleDigests() { $recipient = 'foo@bar.baz'; $nEmailsSent = 15; - $user = UserFactory::make()->withAvatar()->persist(); - $admin = UserFactory::make()->withAvatar()->persist(); + [$user, $admin] = UserFactory::make(2)->withAvatar()->persist(); + EmailQueueFactory::make($nEmailsSent) ->setRecipient($recipient) ->setTemplate(ResourceCreateEmailRedactor::TEMPLATE) @@ -160,17 +164,25 @@ public function testSenderCommandMultipleDigests() ->setField('template_vars.body.user', $user) ->persist(); + // Upgrade priority of this template to ensure that the emails are sent in this order + $priorityResourceChange = rand(); + DigestTemplateRegistry::getInstance()->addTemplate(new ResourceChangesDigestTemplate($priorityResourceChange)); + DigestTemplateRegistry::getInstance()->addTemplate( + new GroupMembershipDigestTemplate($priorityResourceChange + 1) + ); + $this->exec('passbolt email_digest send'); $this->assertExitSuccess(); $this->assertMailCount(2); - - $this->assertMailSubjectContainsAt(0, 'Multiple passwords have been changed in passbolt'); + $subject = $user->profile->full_name . ' has made changes on several resources'; + $this->assertMailSubjectContainsAt(0, $subject); $this->assertMailContainsAt(0, $nEmailsSent . ' resources were affected.'); - $this->assertMailContainsAt(0, 'Edited multiple resources'); + $this->assertMailContainsAt(0, $subject); - $this->assertMailSubjectContainsAt(1, 'Your membership in several groups changed in passbolt'); + $subject = $user->profile->full_name . ' updated your memberships in several groups'; + $this->assertMailSubjectContainsAt(1, $subject); + $this->assertMailContainsAt(1, $subject); $this->assertMailContainsAt(1, $nEmailsSent . ' group memberships were affected.'); - $this->assertMailContainsAt(1, 'Edited your membership in several groups'); } } diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/PreviewEmailBatchServiceTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/PreviewEmailBatchServiceTest.php index d63bf22885..11d8452287 100644 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/PreviewEmailBatchServiceTest.php +++ b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/PreviewEmailBatchServiceTest.php @@ -57,12 +57,12 @@ public function tearDown(): void public function testPreviewNextEmailBatch(): void { $numberOfEmails = 3; - $limit = $numberOfEmails - 1; - EmailQueueFactory::make($numberOfEmails)->persist(); + /** @var \Cake\ORM\Entity[] $emails */ + $emails = EmailQueueFactory::make($numberOfEmails)->getEntities(); - $result = $this->previewEmailBatchService->previewNextEmailsBatch($limit); + $result = $this->previewEmailBatchService->previewNextEmailsBatch($emails); - $this->assertSame($limit, count($result)); + $this->assertSame($numberOfEmails, count($result)); foreach ($result as $email) { $this->assertNotEmpty($email->getHeaders()); $this->assertNotEmpty($email->getContent()); @@ -74,15 +74,16 @@ public function testPreviewNextEmailBatchTranslated(): void $this->loadPlugins(['Passbolt/Locale' => []]); $frenchLocale = 'fr-FR'; + /** @var \App\Model\Entity\User $frenchSpeakingUser */ $frenchSpeakingUser = UserFactory::make()->user()->withLocale($frenchLocale)->persist(); - EmailQueueFactory::make(['created' => Chronos::now()->subDays(2)])->persist(); - EmailQueueFactory::make(['created' => Chronos::now()->subDays(1)]) + $emails[] = EmailQueueFactory::make(['created' => Chronos::now()->subDays(2)])->persist(); + $emails[] = EmailQueueFactory::make(['created' => Chronos::now()->subDays(1)]) ->setRecipient($frenchSpeakingUser->username) ->persist(); - EmailQueueFactory::make(['created' => Chronos::now()])->persist(); + $emails[] = EmailQueueFactory::make(['created' => Chronos::now()])->persist(); - $emailBatch = $this->previewEmailBatchService->previewNextEmailsBatch(); + $emailBatch = $this->previewEmailBatchService->previewNextEmailsBatch($emails); $emailInEnglish1 = $emailBatch[0]; $emailInFrench = $emailBatch[1]; $emailInEnglish2 = $emailBatch[2]; diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceCreateResourceChangesDigestTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceCreateResourceChangesDigestTest.php index 671b6f565e..6aabcb3d73 100644 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceCreateResourceChangesDigestTest.php +++ b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceCreateResourceChangesDigestTest.php @@ -75,7 +75,7 @@ public function testSendEmailBatchServiceCreateResourceChangesDigest_SendNextEma $this->postJson('/resources.json', $data); $this->assertSuccess(); - $this->service->sendNextEmailsBatch(); + $this->service->sendNextEmailsBatch(EmailQueueFactory::find()->all()->toArray()); $this->assertSame(1, ResourceFactory::count()); $this->assertMailCount(1); @@ -98,7 +98,7 @@ public function testSendEmailBatchServiceCreateResourceChangesDigest_SendNextEma $frenchSpeakingUser = UserFactory::make()->user()->withLocale('fr-FR')->persist(); /** @psalm-suppress InternalMethod */ I18n::getTranslator('default', 'fr_FR')->getPackage()->addMessages([ - '{0} has made changes on several resources' => '{0} a apporté des modifications à plusieurs ressources', + 'You made changes on several resources' => 'Vous avez apporté des modifications à plusieurs ressources', 'You have saved a new password' => 'Vous avez enregistré un nouveau mot de passe', ]); @@ -108,19 +108,20 @@ public function testSendEmailBatchServiceCreateResourceChangesDigest_SendNextEma $this->assertSuccess(); } - $this->service->sendNextEmailsBatch(); + $this->service->sendNextEmailsBatch(EmailQueueFactory::find()->all()->toArray()); - $recipientFirstName = $frenchSpeakingUser->profile->first_name; $this->assertSame($nResourcesAdded, ResourceFactory::count()); $this->assertMailCount(1); - $this->assertMailSubjectContainsAt(0, $recipientFirstName . ' a apporté des modifications à plusieurs ressources'); + $this->assertMailSubjectContainsAt(0, 'Vous avez apporté des modifications à plusieurs ressources'); + $this->assertMailContainsAt(0, 'Vous avez apporté des modifications à plusieurs ressources'); $this->assertMailContainsAt(0, 'Vous avez enregistré un nouveau mot de passe'); } public function testSendEmailBatchServiceCreateResourceChangesDigest_SendNextEmailsBatch_Above_Threshold() { $nResourcesAdded = 15; + /** @var \App\Model\Entity\User $operator */ $operator = UserFactory::make()->withAvatar()->persist(); EmailQueueFactory::make($nResourcesAdded) ->setRecipient('foo@bar.baz') @@ -131,7 +132,7 @@ public function testSendEmailBatchServiceCreateResourceChangesDigest_SendNextEma /** @psalm-suppress InternalMethod */ I18n::getTranslator('default', 'fr_FR')->getPackage()->addMessages([ - 'Multiple passwords have been changed in passbolt' => 'Plusieurs mots de passe ont été modifiés dans passbolt', + '{0} has made changes on several resources' => '{0} a apporté des modifications à plusieurs ressources', '{0} resources were affected.' => '{0} ressources ont été affectées.', ]); @@ -139,7 +140,8 @@ public function testSendEmailBatchServiceCreateResourceChangesDigest_SendNextEma $this->assertExitSuccess(); $this->assertMailCount(1); - $this->assertMailSubjectContainsAt(0, 'Plusieurs mots de passe ont été modifiés dans passbolt'); + $this->assertMailSubjectContainsAt(0, $operator->profile->full_name . ' a apporté des modifications à plusieurs ressources'); + $this->assertMailContainsAt(0, $operator->profile->full_name . ' a apporté des modifications à plusieurs ressources'); $this->assertMailContainsAt(0, $nResourcesAdded . ' ressources ont été affectées.'); } } diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceTest.php deleted file mode 100644 index 34c3a15561..0000000000 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceTest.php +++ /dev/null @@ -1,225 +0,0 @@ -loadPlugins(['Passbolt/EmailDigest' => []]); - - // Mock the digest service - $this->emailDigestServiceMock = $this->createMock(EmailDigestService::class); - - // Mock a model, maintain fixtures and table association - $this->emailQueueTableMock = $this->getMockForModel('EmailQueue.EmailQueue', [ - 'getBatch', - 'success', - 'fail', - 'releaseLocks', - ])->setTable('email_queue'); // the CakePHP getMockForModel method use Inflector:tableize() method which make the table name plural - - $this->sut = new SendEmailBatchService( - $this->emailQueueTableMock, - $this->emailDigestServiceMock - ); - } - - public function testSendEmailBatchService_MarkAsSentWhenSendDigestSucceed() - { - // Define the list of email queue entity ids that should be marked as success at the end of the test - $expectedEmailsIdsToUpdate = [[1], [2]]; - - // We create emails entities from email-queue plugin - $emailQueueEntities = [ - $this->createEmailQueueEntity(['id' => 1]), - $this->createEmailQueueEntity(['id' => 2]), - ]; - - // Simulate the getBatch method from the EmailQueue model - $this->emailQueueTableMock->expects($this->once()) - ->method('getBatch') - ->willReturn($emailQueueEntities); - - // We mock the digest services and make it returns digests - // It is normally returned by the digests but we directly create them in this situation - // because we do not need to test the marshalling logic in this test which is a separate concern) - $this->emailDigestServiceMock->expects($this->once()) - ->method('createDigests') - ->willReturn($this->createEmailDigestsFromEmailEntities($emailQueueEntities)); - - // It should call the success method of the email queue for each emails - $this->assertEmailQueueWillFlagEmailsAsSuccess($expectedEmailsIdsToUpdate); - - $this->sut->sendNextEmailsBatch(); - } - - public function testSendEmailBatchService_MarkAsNonSentWhenSendDigestFailed() - { - $this->makeEmailTransportFailWithException(new SocketException('Failed to send email.')); - - // Define the list of email queue entity ids that should be marked as success at the end of the test - $expectedIdFailureMessageCouples = [ - [1, 'Failed to send email.'], - [2, 'Failed to send email.'], - ]; - - // We create emails entities from email-queue plugin - $emailQueueEntities = [ - $this->createEmailQueueEntity(['id' => 1]), - $this->createEmailQueueEntity(['id' => 2]), - ]; - - // Simulate the getBatch method from the EmailQueue model - $this->emailQueueTableMock->expects($this->once()) - ->method('getBatch') - ->willReturn($emailQueueEntities); - - // We mock the digest services and make it returns our digests initialized earlier - // It is normally returned by the digests but we directly create them in this situation - // because we do not need to test the marshalling logic in this test) - $this->emailDigestServiceMock->expects($this->once()) - ->method('createDigests') - ->willReturn($this->createEmailDigestsFromEmailEntities($emailQueueEntities)); - - $this->assertEmailQueueWillFlagEmailsAsFailed($expectedIdFailureMessageCouples); - - $this->sut->sendNextEmailsBatch(); - } - - public function testSendEmailBatchService_ReleaseLocksAfterSend() - { - // Define the list of email queue entity ids that should be marked as success at the end of the test - $expectedEmailsIdsToUpdate = [1, 2]; - - // We create emails entities from email-queue plugin - $emailQueueEntities = [ - $this->createEmailQueueEntity(['id' => 1]), - $this->createEmailQueueEntity(['id' => 2]), - ]; - - // Simulate the getBatch method from the EmailQueue model - $this->emailQueueTableMock->expects($this->once()) - ->method('getBatch') - ->willReturn($emailQueueEntities); - - // We mock the digest services and make it returns our digests initialized earlier - // It is normally returned by the digests but we directly create them in this situation - // because we do not need to test the marshalling logic in this test) - $this->emailDigestServiceMock->expects($this->once()) - ->method('createDigests') - ->willReturn($this->createEmailDigestsFromEmailEntities($emailQueueEntities)); - - // It should call the releaseLocks method with the ids of every emails which have been sent with or without success - $this->assertEmailQueueWillReleaseLocksForEmailIds($expectedEmailsIdsToUpdate); - - $this->sut->sendNextEmailsBatch(); - } - - private function assertEmailQueueWillFlagEmailsAsSuccess(array $expectedIdWithSuccess) - { - // It should call the `success` method of the email queue for each emails - $this->emailQueueTableMock - ->expects($this->exactly(count($expectedIdWithSuccess))) - ->method('success') - ->withConsecutive(...$expectedIdWithSuccess); - } - - private function assertEmailQueueWillFlagEmailsAsFailed(array $expectedIdFailureMessageCouples) - { - // It should call the `fail` method of the email queue for each emails - $this->emailQueueTableMock - ->expects($this->exactly(count($expectedIdFailureMessageCouples))) - ->method('fail') - ->withConsecutive(...$expectedIdFailureMessageCouples); - } - - private function assertEmailQueueWillReleaseLocksForEmailIds(array $emailIds) - { - $this->emailQueueTableMock - ->expects($this->once()) - ->method('releaseLocks') - ->with($emailIds); - } - - private function createEmailDigestsFromEmailEntities(array $emailEntities) - { - return [$this->createEmailDigest($emailEntities)]; - } - - /** - * Replace the transport method for the email with a Transport class which raises an exception when sending the email. - * This is useful to simulate a failure during send and assert on that. - */ - private function makeEmailTransportFailWithException(Throwable $exception) - { - $configuredTransports = TransportFactory::configured(); - foreach ($configuredTransports as $configuredTransport) { - $config = TransportFactory::getConfig($configuredTransport); - $config['className'] = self::class; - $instance = new class ([], $exception) extends TestEmailTransport - { - /** - * @var mixed - */ - private $exception; - - public function __construct($config, $exception) - { - $this->exception = $exception; - - parent::__construct($config); - } - - public function send(Message $message): array - { - throw $this->exception; - } - }; - TransportFactory::drop($configuredTransport); - TransportFactory::setConfig($configuredTransport, $instance); - } - } -} diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceUnitTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceUnitTest.php new file mode 100644 index 0000000000..9e9cb665fd --- /dev/null +++ b/plugins/PassboltCe/EmailDigest/tests/TestCase/Service/SendEmailBatchServiceUnitTest.php @@ -0,0 +1,442 @@ +service = new SendEmailBatchService(); + $this->loadPlugins([EmailDigestPlugin::class => []]); + $this->loadPlugins([LocalePlugin::class => []]); + (new AvatarsConfigurationService())->loadConfiguration(); + /** @psalm-suppress InternalMethod */ + I18n::getTranslator('default', 'fr_FR')->getPackage()->addMessages([ + 'Name: {0}' => 'Nom: {0}', + '{0} has made changes on several resources' => '{0} a apporté des modifications à plusieurs ressources', + 'You made changes on several resources' => 'Vous avez apporté des modifications à plusieurs ressources', + '{0} resources were affected.' => '{0} ressources ont été affectées.', + '{0} removed you from the group {1}' => '{0} vous a retiré du groupe {1}', + ]); + } + + public function tearDown(): void + { + unset($this->service); + parent::tearDown(); + } + + public function testSendEmailBatchService_On_No_Email() + { + $this->service->sendNextEmailsBatch([]); + $this->assertMailCount(0); + } + + public function testSendEmailBatchService_On_Email_With_Unknown_Template() + { + $email = EmailQueueFactory::make()->setTemplate('foo')->getEntity(); + $this->expectException(MissingTemplateException::class); + $this->service->sendNextEmailsBatch([$email]); + } + + /** + * Run tests twice, once with email digest activated, once not + * + * @return array + */ + public function withAndWithoutDigestTemplate(): array + { + return [[true], [false]]; + } + + /** + * @dataProvider withAndWithoutDigestTemplate + */ + public function testSendEmailBatchService_On_One_Email_Translated(bool $withDigestTemplate) + { + if (!$withDigestTemplate) { + DigestTemplateRegistry::clearInstance(); + } + $operator = UserFactory::make()->withAvatarNull()->getEntity(); + $resourceDeleted = ResourceFactory::make()->getEntity(); + $subjectTranslated = 'Le sujet'; + $email = ResourceDeleteEmailQueueFactory::make() + ->setOperator($operator) + ->setResource($resourceDeleted) + ->setSubject($subjectTranslated) + ->setLocale('fr-FR') + ->getEntity(); + + $this->service->sendNextEmailsBatch([$email]); + + $this->assertMailCount(1); + $this->assertMailSentToAt(0, [$email->get('email') => $email->get('email')]); + $this->assertMailSubjectContainsAt(0, $subjectTranslated); + $this->assertMailContainsAt(0, $subjectTranslated); + $this->assertMailContainsAt(0, 'Nom: ' . $resourceDeleted->name); + } + + /** + * @dataProvider withAndWithoutDigestTemplate + */ + public function testSendEmailBatchService_On_Multiple_Emails_Same_Recipient_Below_Threshold( + bool $withDigestTemplate + ) { + if (!$withDigestTemplate) { + DigestTemplateRegistry::clearInstance(); + } + $operator = UserFactory::make()->withAvatarNull()->getEntity(); + $operator->profile->avatar = null; + $resourceDeleted = ResourceFactory::make()->getEntity(); + $subjectTranslated = 'Le sujet'; + $recipient = 'foo@passbolt.com'; + $nEmails = rand(2, 10); + $emails = ResourceDeleteEmailQueueFactory::make($nEmails) + ->setRecipient($recipient) + ->setOperator($operator) + ->setResource($resourceDeleted) + ->setSubject($subjectTranslated) + ->setLocale('fr-FR') + ->getEntities(); + + $this->service->sendNextEmailsBatch($emails); + + if ($withDigestTemplate) { + $this->assertMailCount(1); + $this->assertMailSentToAt(0, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(0, $operator->profile->full_name . ' a apporté des modifications à plusieurs ressources'); + $this->assertMailContainsAt(0, $subjectTranslated); + $this->assertMailContainsAt(0, 'Nom: ' . $resourceDeleted->name); + } else { + $this->assertMailCount($nEmails); + foreach ($emails as $i => $email) { + $this->assertMailSentToAt($i, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt($i, $subjectTranslated); + $this->assertMailContainsAt($i, $subjectTranslated); + $this->assertMailContainsAt($i, 'Nom: ' . $resourceDeleted->name); + } + } + } + + /** + * @dataProvider withAndWithoutDigestTemplate + */ + public function testSendEmailBatchService_On_Same_Digest_Template_Various_Recipients( + bool $withDigestTemplate + ) { + if (!$withDigestTemplate) { + DigestTemplateRegistry::clearInstance(); + } + $recipient1 = 'foo@passbolt.com'; + $recipient2 = 'bar@passbolt.com'; + $operator = UserFactory::make()->withAvatarNull()->getEntity(); + [$resourceDeleted1, $resourceDeleted2] = ResourceFactory::make(2)->getEntities(); + $subject = 'Subject'; + $nEmails1 = rand(2, 10); + $nEmails2 = rand(2, 10); + $emails1 = ResourceDeleteEmailQueueFactory::make($nEmails1) + ->setRecipient($recipient1) + ->setOperator($operator) + ->setResource($resourceDeleted1) + ->setSubject($subject) + ->getEntities(); + $emails2 = ResourceDeleteEmailQueueFactory::make($nEmails2) + ->setRecipient($recipient2) + ->setOperator($operator) + ->setResource($resourceDeleted2) + ->setSubject($subject) + ->getEntities(); + + $allEmails = array_merge($emails1, $emails2); + + $this->service->sendNextEmailsBatch($allEmails); + + if ($withDigestTemplate) { + $this->assertMailCount(2); + $this->assertMailSentToAt(0, [$recipient1 => $recipient1]); + $this->assertMailSubjectContainsAt(0, $operator->profile->full_name . ' has made changes on several resources'); + $this->assertMailContainsAt(0, $subject); + $this->assertMailContainsAt(0, 'Name: ' . $resourceDeleted1->name); + + $this->assertMailSentToAt(1, [$recipient2 => $recipient2]); + $this->assertMailSubjectContainsAt(1, $operator->profile->full_name . ' has made changes on several resources'); + $this->assertMailContainsAt(1, $subject); + $this->assertMailContainsAt(1, 'Name: ' . $resourceDeleted2->name); + } else { + $this->assertMailCount(count($allEmails)); + foreach ($emails1 as $i => $email) { + $this->assertMailSentToAt($i, [$recipient1 => $recipient1]); + $this->assertMailSubjectContainsAt($i, $subject); + $this->assertMailContainsAt($i, $subject); + $this->assertMailContainsAt($i, 'Name: ' . $resourceDeleted1->name); + } + + foreach ($emails2 as $i => $email) { + $index = (int)$i + $nEmails1; + $this->assertMailSentToAt($index, [$recipient2 => $recipient2]); + $this->assertMailSubjectContainsAt($index, $subject); + $this->assertMailContainsAt($index, $subject); + $this->assertMailContainsAt($index, 'Name: ' . $resourceDeleted2->name); + } + } + } + + /** + * @dataProvider withAndWithoutDigestTemplate + */ + public function testSendEmailBatchService_On_Multiple_Emails_Multiple_Operators_Below_Threshold( + bool $withDigestTemplate + ) { + if (!$withDigestTemplate) { + DigestTemplateRegistry::clearInstance(); + } + $recipient = 'foo@passbolt.com'; + // Operator 1 is the recipient too. This changes the subject + [$operator1, $operator2] = UserFactory::make([['username' => $recipient], []])->withAvatarNull()->getEntities(); + [$resourceDeleted1, $resourceDeleted2] = ResourceFactory::make(2)->getEntities(); + $subjectTranslated = 'Le sujet'; + $nEmails1 = rand(2, 10); + $nEmails2 = rand(2, 10); + $emails1 = ResourceDeleteEmailQueueFactory::make($nEmails1) + ->setRecipient($recipient) + ->setOperator($operator1) + ->setResource($resourceDeleted1) + ->setSubject($subjectTranslated) + ->setLocale('fr-FR') + ->getEntities(); + $emails2 = ResourceDeleteEmailQueueFactory::make($nEmails2) + ->setRecipient($recipient) + ->setOperator($operator2) + ->setResource($resourceDeleted2) + ->setSubject($subjectTranslated) + ->setLocale('fr-FR') + ->getEntities(); + + $allEmails = array_merge($emails1, $emails2); + + $this->service->sendNextEmailsBatch($allEmails); + + if ($withDigestTemplate) { + $this->assertMailCount(2); + $this->assertMailSentToAt(0, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(0, 'Vous avez apporté des modifications à plusieurs ressources'); + $this->assertMailContainsAt(0, $subjectTranslated); + $this->assertMailContainsAt(0, 'Nom: ' . $resourceDeleted1->name); + + $this->assertMailSentToAt(1, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(1, $operator2->profile->full_name . ' a apporté des modifications à plusieurs ressources'); + $this->assertMailContainsAt(1, $subjectTranslated); + $this->assertMailContainsAt(1, 'Nom: ' . $resourceDeleted2->name); + } else { + $this->assertMailCount(count($allEmails)); + foreach ($emails1 as $i => $email) { + $this->assertMailSentToAt($i, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt($i, $subjectTranslated); + $this->assertMailContainsAt($i, $subjectTranslated); + $this->assertMailContainsAt($i, 'Nom: ' . $resourceDeleted1->name); + } + + foreach ($emails2 as $i => $email) { + $index = (int)$i + $nEmails1; + $this->assertMailSentToAt($index, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt($index, $subjectTranslated); + $this->assertMailContainsAt($index, $subjectTranslated); + $this->assertMailContainsAt($index, 'Nom: ' . $resourceDeleted2->name); + } + } + } + + /** + * @dataProvider withAndWithoutDigestTemplate + */ + public function testSendEmailBatchService_On_Multiple_Emails_Multiple_Operators_Below_And_Above_Threshold( + bool $withDigestTemplate + ) { + if (!$withDigestTemplate) { + DigestTemplateRegistry::clearInstance(); + } + [$operator1, $operator2] = UserFactory::make(2)->withAvatarNull()->getEntities(); + [$resourceDeleted1, $resourceDeleted2] = ResourceFactory::make(2)->getEntities(); + $subjectTranslated = 'Le sujet'; + $recipient = 'foo@passbolt.com'; + $nEmails1 = 10 + rand(2, 10); + $nEmails2 = rand(2, 10); + // These emails are above the threshold + $emails1 = ResourceDeleteEmailQueueFactory::make($nEmails1) + ->setRecipient($recipient) + ->setOperator($operator1) + ->setResource($resourceDeleted1) + ->setSubject($subjectTranslated) + ->setLocale('fr-FR') + ->getEntities(); + $emails2 = ResourceDeleteEmailQueueFactory::make($nEmails2) + ->setRecipient($recipient) + ->setOperator($operator2) + ->setResource($resourceDeleted2) + ->setSubject($subjectTranslated) + ->setLocale('fr-FR') + ->getEntities(); + + $allEmails = array_merge($emails1, $emails2); + + $this->service->sendNextEmailsBatch($allEmails); + + if ($withDigestTemplate) { + $this->assertMailCount(2); + $this->assertMailSentToAt(0, [$recipient => $recipient]); + $subject = $operator1->profile->full_name . ' a apporté des modifications à plusieurs ressources'; + $this->assertMailSubjectContainsAt(0, $subject); + $this->assertMailContainsAt(0, $subject); + $this->assertMailContainsAt(0, $nEmails1 . ' ressources ont été affectées.'); + + $this->assertMailSentToAt(1, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(1, $operator2->profile->full_name . ' a apporté des modifications à plusieurs ressources'); + $this->assertMailContainsAt(1, $subjectTranslated); + $this->assertMailContainsAt(1, 'Nom: ' . $resourceDeleted2->name); + } else { + $this->assertMailCount(count($allEmails)); + foreach ($emails1 as $i => $email) { + $this->assertMailSentToAt($i, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt($i, $subjectTranslated); + $this->assertMailContainsAt($i, $subjectTranslated); + $this->assertMailContainsAt($i, 'Nom: ' . $resourceDeleted1->name); + } + + foreach ($emails2 as $i => $email) { + $index = (int)$i + $nEmails1; + $this->assertMailSentToAt($index, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt($index, $subjectTranslated); + $this->assertMailContainsAt($index, $subjectTranslated); + $this->assertMailContainsAt($index, 'Nom: ' . $resourceDeleted2->name); + } + } + } + + public function testSendEmailBatchService_On_Multiple_Emails_Multiple_Operators_Multiple_Digest_Templates_Below_And_Above_Threshold() + { + DigestTemplateRegistry::clearInstance(); + // Emails of the Group User delete template should be sent first + DigestTemplateRegistry::getInstance() + ->addTemplate(new GroupUserDeleteDigestTemplate(1)) + ->addTemplate((new ResourceChangesDigestTemplate())); + + [$operator1, $operator2] = UserFactory::make(2) + ->withAvatarNull() + ->getEntities(); + [$resourceDeleted1, $resourceDeleted2] = ResourceFactory::make(2)->getEntities(); + [$groupDeleted1, $groupDeleted2] = GroupFactory::make(2)->getEntities(); + $subjectTranslated = 'Le sujet'; + $recipient = 'foo@passbolt.com'; + $nEmailsResourceDeleted1 = rand(11, 20); + $nEmailsResourceDeleted2 = rand(11, 20); + $nEmailsGroupDeleted1 = rand(11, 20); + $nEmailsGroupDeleted2 = rand(11, 20); + + // These emails are above the threshold + $emailsResourceDeleted1 = ResourceDeleteEmailQueueFactory::make($nEmailsResourceDeleted1) + ->setRecipient($recipient) + ->setOperator($operator1) + ->setResource($resourceDeleted1) + ->setSubject($subjectTranslated) + ->getEntities(); + $emailsResourceDeleted2 = ResourceDeleteEmailQueueFactory::make($nEmailsResourceDeleted2) + ->setRecipient($recipient) + ->setOperator($operator2) + ->setResource($resourceDeleted2) + ->setSubject($subjectTranslated) + ->getEntities(); + $emailsGroupDeleted1 = GroupUserDeleteEmailQueueFactory::make($nEmailsGroupDeleted1) + ->setRecipient($recipient) + ->setOperator($operator1) + ->setGroup($groupDeleted1) + ->setSubject($subjectTranslated) + ->getEntities(); + $emailsGroupDeleted2 = GroupUserDeleteEmailQueueFactory::make($nEmailsGroupDeleted2) + ->setRecipient($recipient) + ->setOperator($operator2) + ->setGroup($groupDeleted2) + ->setSubject($subjectTranslated) + ->getEntities(); + + $allEmails = array_merge( + $emailsResourceDeleted1, + $emailsResourceDeleted2, + $emailsGroupDeleted1, + $emailsGroupDeleted2 + ); + + $this->service->sendNextEmailsBatch($allEmails); + + $this->assertMailCount(4); + $this->assertMailSentToAt(0, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(0, $operator1->profile->full_name . ' deleted several group memberships'); + $this->assertMailSentToAt(1, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(1, $operator2->profile->full_name . ' deleted several group memberships'); + $this->assertMailSentToAt(2, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(2, $operator1->profile->full_name . ' has made changes on several resources'); + $this->assertMailSentToAt(3, [$recipient => $recipient]); + $this->assertMailSubjectContainsAt(3, $operator2->profile->full_name . ' has made changes on several resources'); + } + + public function testSendEmailBatchService_On_Multiple_Full_Base_Url() + { + $recipient = 'test@passbolt.com'; + $subject = 'Some subject'; + $operator = UserFactory::make()->withAvatarNull()->getEntity(); + $emails1 = ResourceDeleteEmailQueueFactory::make(2) + ->setRecipient($recipient) + ->setOperator($operator) + ->setFullBaseUrl('foo') + ->getEntities(); + + $emails2 = ResourceDeleteEmailQueueFactory::make(2) + ->setRecipient($recipient) + ->setOperator($operator) + ->setFullBaseUrl('bar') + ->getEntities(); + + $allEmails = array_merge($emails1, $emails2); + + $this->service->sendNextEmailsBatch($allEmails); + + $this->assertMailCount(2); + } +} diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsCollectionTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsCollectionTest.php deleted file mode 100644 index 12885089a8..0000000000 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsCollectionTest.php +++ /dev/null @@ -1,165 +0,0 @@ -digestsCollectionMock = $this->createMock(DigestsPool::class); - - $this->sut = new DigestsCollection($this->digestsCollectionMock); - } - - public function testDigestsCollection_canAddToDigestSuccess() - { - $emailEntity = $this->createEmailQueueEntity(); - - // We create a digest lambda which can support the email entity - $this->digestsCollectionMock->expects($this->once()) - ->method('getDigests') - ->willReturn([ - $this->createDigest(true, []), - ]); - - $this->assertTrue($this->sut->canAddToDigest($emailEntity), 'Marshaller should support the given email.'); - } - - public function testDigestsCollection_canAddToDigestFail() - { - $emailEntity = $this->createEmailQueueEntity(); - - // We create a digest lambda which can support the email entity - $this->digestsCollectionMock->expects($this->once()) - ->method('getDigests') - ->willReturn([ - $this->createDigest(false, []), - ]); - - $this->assertFalse($this->sut->canAddToDigest($emailEntity), 'Marshaller should not support the given email.'); - } - - /** - * @throws UnsupportedEmailDigestDataException - */ - public function testDigestsCollection_AddEmailEntityToFirstValidDigestInThePool() - { - $emailEntity = $this->createEmailQueueEntity(); - - // We initialize 2 digests, only 1 of them is expected to be called - $digestThatShouldBeCalled = $this->createMock(DigestInterface::class); - $digestThatShouldNotBeCalled = $this->createMock(DigestInterface::class); - - $this->digestsCollectionMock->expects($this->once()) - ->method('getDigests') - ->willReturn([ - $digestThatShouldBeCalled, - $digestThatShouldNotBeCalled, - ]); - - // We set expectations on how the defined digests should be called - $this->assertDigestWillAddEmailEntity($digestThatShouldBeCalled, $emailEntity); - $this->assertDigestWillNeverAddEmailEntity($digestThatShouldNotBeCalled, $emailEntity); - - $this->sut->addEmailEntity($emailEntity); - } - - public function testDigestsCollection_ThrowUnsupportedEmailDigestException() - { - $emailEntity = $this->createEmailQueueEntity(); - - // We create a digest lambda which can support the email entity - $this->digestsCollectionMock->expects($this->once()) - ->method('getDigests') - ->willReturn([ - $this->createDigest(false, []), - ]); - - $this->expectException(UnsupportedEmailDigestDataException::class); - - $this->sut->addEmailEntity($emailEntity); - } - - public function testDigestsCollection_MergeEmailDigestsFromDifferentDigestTogether() - { - $emailDigestsFromDigestA = [ - $this->createEmailDigest(), - ]; - $emailDigestsFromDigestB = [ - $this->createEmailDigest(), - ]; - - // We create 2 digests which return their emails - $this->digestsCollectionMock->expects($this->once()) - ->method('getDigests') - ->willReturn([ - $this->createDigest(true, $emailDigestsFromDigestA), - $this->createDigest(true, $emailDigestsFromDigestB), - ]); - - $expectedDigests = array_merge($emailDigestsFromDigestA, $emailDigestsFromDigestB); - - $this->assertSame($expectedDigests, $this->sut->marshalEmails()); - } - - private function assertDigestWillAddEmailEntity(MockObject $digest, Entity $emailEntity) - { - $digest->expects($this->once()) - ->method('addEmailEntity') - ->with($emailEntity) - ->willReturn($digest); - - $digest->expects($this->once()) - ->method('canAddToDigest') - ->willReturn(true); - } - - private function assertDigestWillNeverAddEmailEntity(MockObject $digest, Entity $emailEntity) - { - $digest->expects($this->never()) - ->method('addEmailEntity') - ->with($emailEntity) - ->willReturn($digest); - - $digest->expects($this->never()) - ->method('canAddToDigest') - ->willReturn(true); - } -} diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsPoolTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsPoolTest.php deleted file mode 100644 index 4680b722df..0000000000 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/DigestsPoolTest.php +++ /dev/null @@ -1,72 +0,0 @@ -digestsPool = DigestsPool::getInstance(); - parent::setUp(); - } - - public function tearDown(): void - { - unset($this->digestsPool); - DigestsPool::clearInstance(); - parent::tearDown(); - } - - public function testEmailDigestDigestsPoolAddDigest() - { - // Create a lambda digest - $digest = $this->createDigest(true, []); - - $this->digestsPool->addDigest($digest); - - // Assert that digest was added with success - $this->assertContains($digest, $this->digestsPool->getDigests()); - } - - public function testEmailDigestDigestsPoolGetDigest() - { - $digestsPool = DigestsPool::getInstance(); - - $digestWithHighestPriority = $this->createDigest(true, []); - $digestWithLowestPriority = $this->createDigest(true, []); - - $this->digestsPool->addDigest($digestWithHighestPriority, 2); - $this->digestsPool->addDigest($digestWithLowestPriority, 1); - - $digests = $digestsPool->getDigests(); - - $this->assertSame($digestWithHighestPriority, $digests[0]); - $this->assertSame($digestWithLowestPriority, $digests[1]); - } -} diff --git a/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/SingleEmailDigestTest.php b/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/SingleEmailDigestTest.php deleted file mode 100644 index 6e4fabb98f..0000000000 --- a/plugins/PassboltCe/EmailDigest/tests/TestCase/Utility/Digest/SingleEmailDigestTest.php +++ /dev/null @@ -1,84 +0,0 @@ -emailPreviewFactoryMock = $this->createMock(EmailPreviewFactory::class); - $this->sut = new SingleDigest($this->emailPreviewFactoryMock); - } - - public function testSingleEmailDigest_CanAddToDigestAlwaysTrue() - { - $this->assertTrue($this->sut->canAddToDigest($this->createEmailQueueEntity())); - } - - public function testSingleEmailDigest_AddEmailEntityReturnInstanceOfItself() - { - $emailEntities = [ - $this->createEmailQueueEntity(), - $this->createEmailQueueEntity(), - $this->createEmailQueueEntity(), - ]; - foreach ($emailEntities as $emailEntity) { - $this->assertSame($this->sut, $this->sut->addEmailEntity($emailEntity)); - } - } - - public function testSingleEmailDigest_CreateAsManyEmailDigestsAsThereAreEmails() - { - $emailEntities = [ - $this->createEmailQueueEntity(), - $this->createEmailQueueEntity(), - $this->createEmailQueueEntity(), - ]; - foreach ($emailEntities as $emailEntity) { - $this->sut->addEmailEntity($emailEntity); - } - - $this->emailPreviewFactoryMock->expects($this->exactly(3)) - ->method('renderFromEmailEntity') - ->willReturn('content'); - - $expectedEmailDigestsCount = count($emailEntities); - - $digests = $this->sut->marshalEmails(); - - $this->assertCount($expectedEmailDigestsCount, $digests); - } -} diff --git a/psalm-baseline-v5-upgrade.xml b/psalm-baseline-v5-upgrade.xml index 87ebca63a6..89101313ca 100644 --- a/psalm-baseline-v5-upgrade.xml +++ b/psalm-baseline-v5-upgrade.xml @@ -5,33 +5,6 @@ id]]> - - - DigestRegisterEvent - - - - - $sender[$senderEmail] - - - $senderEmail - $senderEmail - - - username]]> - - - - - username]]> - - - - - username]]> - - EmailNotificationSettingsDefinitionRegisterEvent diff --git a/src/Notification/DigestTemplate/GroupMembershipDigestTemplate.php b/src/Notification/DigestTemplate/GroupMembershipDigestTemplate.php new file mode 100644 index 0000000000..8f13a91973 --- /dev/null +++ b/src/Notification/DigestTemplate/GroupMembershipDigestTemplate.php @@ -0,0 +1,72 @@ +logErrorIfTheRecipientCannotBeTheOperator(); + } + + /** + * @inheritDoc + */ + public function getDigestSubjectIfRecipientIsNotTheOperator(): string + { + return __('{0} updated your memberships in several groups', '{0}'); + } + + /** + * @inheritDoc + */ + public function getSupportedTemplates(): array + { + return [ + GroupUserAddEmailRedactor::TEMPLATE, + GroupUserUpdateEmailRedactor::TEMPLATE, + GroupUserDeleteEmailRedactor::TEMPLATE, + ]; + } + + /** + * @inheritDoc + */ + public function getOperatorVariableKey(): string + { + return 'user'; + } + + /** + * @inheritDoc + */ + public function getDigestTemplate(): string + { + return self::GROUP_USERS_CHANGES_TEMPLATE; + } +} diff --git a/src/Notification/DigestTemplate/GroupUserDeleteDigestTemplate.php b/src/Notification/DigestTemplate/GroupUserDeleteDigestTemplate.php new file mode 100644 index 0000000000..57434fcb07 --- /dev/null +++ b/src/Notification/DigestTemplate/GroupUserDeleteDigestTemplate.php @@ -0,0 +1,68 @@ +logErrorIfTheRecipientCannotBeTheOperator(); + } + + /** + * @inheritDoc + */ + public function getDigestSubjectIfRecipientIsNotTheOperator(): string + { + return __('{0} deleted several group memberships', '{0}'); + } + + /** + * @inheritDoc + */ + public function getSupportedTemplates(): array + { + return [ + GroupUserDeleteEmailRedactor::TEMPLATE, + ]; + } + + /** + * @inheritDoc + */ + public function getOperatorVariableKey(): string + { + return 'admin'; + } + + /** + * @inheritDoc + */ + public function getDigestTemplate(): string + { + return static::GROUPS_DELETE_TEMPLATE; + } +} diff --git a/src/Notification/DigestTemplate/ResourceChangesDigestTemplate.php b/src/Notification/DigestTemplate/ResourceChangesDigestTemplate.php new file mode 100644 index 0000000000..7857788afa --- /dev/null +++ b/src/Notification/DigestTemplate/ResourceChangesDigestTemplate.php @@ -0,0 +1,76 @@ +logErrorIfTheRecipientCannotBeTheOperator(); + } + + /** + * @inheritDoc + */ + public function getDigestSubjectIfRecipientIsNotTheOperator(): string + { + return __('{0} shared several items with you', '{0}'); + } + + /** + * @inheritDoc + */ + public function getSupportedTemplates(): array + { + return [ + ShareEmailRedactor::TEMPLATE, + ]; + } + + /** + * @inheritDoc + */ + public function getOperatorVariableKey(): string + { + return 'owner'; + } + + /** + * @inheritDoc + */ + public function getDigestTemplate(): string + { + return static::RESOURCE_SHARE_MULTIPLE_TEMPLATE; + } +} diff --git a/src/Notification/Email/Redactor/Group/GroupUserDeleteEmailRedactor.php b/src/Notification/Email/Redactor/Group/GroupUserDeleteEmailRedactor.php index cb75acc9ed..85c15288cb 100644 --- a/src/Notification/Email/Redactor/Group/GroupUserDeleteEmailRedactor.php +++ b/src/Notification/Email/Redactor/Group/GroupUserDeleteEmailRedactor.php @@ -25,6 +25,7 @@ use App\Notification\Email\SubscribedEmailRedactorInterface; use App\Notification\Email\SubscribedEmailRedactorTrait; use App\Service\Groups\GroupsUpdateService; +use App\Utility\Purifier; use Cake\Event\Event; use Cake\ORM\TableRegistry; use Cake\Utility\Hash; @@ -127,7 +128,11 @@ private function createGroupUserDeleteEmail(User $recipient, User $admin, Group $subject = (new LocaleService())->translateString( $recipient->locale, function () use ($admin, $group) { - return __('{0} removed you from the group {1}', $admin->profile->first_name, $group->name); + return __( + '{0} removed you from the group {1}', + Purifier::clean($admin['profile']['first_name']), + Purifier::clean($group['name']) + ); } ); $data = ['body' => ['admin' => $admin, 'group' => $group], 'title' => $subject]; diff --git a/src/Notification/Email/Redactor/Resource/ResourceDeleteEmailRedactor.php b/src/Notification/Email/Redactor/Resource/ResourceDeleteEmailRedactor.php index 2a44d51e80..0f887b3ca0 100644 --- a/src/Notification/Email/Redactor/Resource/ResourceDeleteEmailRedactor.php +++ b/src/Notification/Email/Redactor/Resource/ResourceDeleteEmailRedactor.php @@ -25,6 +25,7 @@ use App\Notification\Email\EmailCollection; use App\Notification\Email\SubscribedEmailRedactorInterface; use App\Notification\Email\SubscribedEmailRedactorTrait; +use App\Utility\Purifier; use Cake\Event\Event; use Cake\ORM\TableRegistry; use Passbolt\Locale\Service\LocaleService; @@ -105,12 +106,17 @@ private function createDeleteEmail(User $recipient, User $owner, Resource $resou $subject = (new LocaleService())->translateString( $recipient->locale, function () use ($owner, $resource) { - return __('{0} deleted the password {1}', $owner->profile->first_name, $resource->name); + return __( + '{0} deleted the password {1}', + Purifier::clean($owner->profile->first_name), + Purifier::clean($resource->name) + ); } ); $data = [ 'body' => [ 'user' => $owner, + 'subject' => $subject, 'resource' => $resource, 'showUsername' => $this->getConfig('show.username'), 'showUri' => $this->getConfig('show.uri'), diff --git a/src/Notification/EmailDigest/DigestRegister/GroupDigests.php b/src/Notification/EmailDigest/DigestRegister/GroupDigests.php deleted file mode 100644 index d9a1c91260..0000000000 --- a/src/Notification/EmailDigest/DigestRegister/GroupDigests.php +++ /dev/null @@ -1,121 +0,0 @@ -addDigest($this->createGroupMembershipDigest()); - $digestsPool->addDigest($this->createGroupDeleteDigest()); - } - - /** - * @return \Passbolt\EmailDigest\Utility\Digest\Digest - */ - private function createGroupMembershipDigest(): Digest - { - return new Digest( - __('{0} updated your memberships in several groups', '{0}'), - [ - GroupUserAddEmailRedactor::TEMPLATE, - GroupUserUpdateEmailRedactor::TEMPLATE, - GroupUserDeleteEmailRedactor::TEMPLATE, - ], - 'admin', - /** - * @param \Cake\ORM\Entity[] $emailQueueEntities - */ - function (array $emailQueueEntities, int $emailCount) { - $emailData = $emailQueueEntities[0]; - - $locale = $emailData->get('template_vars')['locale']; - $subject = (new LocaleService())->translateString( - $locale, - function () { - return __('Your membership in several groups changed in passbolt'); - } - ); - - return (new EmailDigest()) - ->setSubject($subject) - ->addLayoutVar(LocaleEmailQueueListener::VIEW_VAR_KEY, $locale) - ->setTemplate(static::GROUP_USERS_CHANGES_TEMPLATE) - ->setEmailRecipient($emailData->get('email')) - ->addTemplateVar('user', $emailData->get('template_vars')['body']['user']) - ->addTemplateVar('fullBaseUrl', Configure::read('App.fullBaseUrl')) - ->addTemplateVar('count', $emailCount); - } - ); - } - - /** - * @return \Passbolt\EmailDigest\Utility\Digest\Digest - */ - private function createGroupDeleteDigest(): Digest - { - return new Digest( - __('{0} deleted several groups', '{0}'), - [ - GroupUserDeleteEmailRedactor::TEMPLATE, - ], - 'admin', - /** - * @param \Cake\ORM\Entity[] $emailQueueEntities - */ - function (array $emailQueueEntities, int $emailCount) { - $emailData = $emailQueueEntities[0]; - - $locale = $emailData->get('template_vars')['locale']; - $subject = (new LocaleService())->translateString( - $locale, - function () { - return __('Several groups were deleted in passbolt'); - } - ); - - return (new EmailDigest()) - ->setSubject($subject) - ->addLayoutVar(LocaleEmailQueueListener::VIEW_VAR_KEY, $locale) - ->setTemplate(static::GROUPS_DELETE_TEMPLATE) - ->setEmailRecipient($emailData->get('email')) - ->addTemplateVar('user', $emailData->get('template_vars')['body']['user']) - ->addTemplateVar('fullBaseUrl', Configure::read('App.fullBaseUrl')) - ->addTemplateVar('count', $emailCount); - } - ); - } -} diff --git a/src/Notification/EmailDigest/DigestRegister/ResourceDigests.php b/src/Notification/EmailDigest/DigestRegister/ResourceDigests.php deleted file mode 100644 index 3ba8b265ca..0000000000 --- a/src/Notification/EmailDigest/DigestRegister/ResourceDigests.php +++ /dev/null @@ -1,148 +0,0 @@ -addDigest($this->createResourceShareDigest()); - $digestsPool->addDigest($this->createResourceChangesDigest()); - } - - /** - * Create a new digest for all emails related to changes on resources. - * It will aggregate the emails for Create, Update and Delete operations in the same digest. - * - * The marshaller will create a digest only if there is minimum 2 emails. - * It will create another digest if there is more than 50 emails. - * - * @return \Passbolt\EmailDigest\Utility\Digest\Digest - */ - private function createResourceChangesDigest(): Digest - { - return new Digest( - __('{0} has made changes on several resources', '{0}'), - [ - ResourceCreateEmailRedactor::TEMPLATE, - ResourceUpdateEmailRedactor::TEMPLATE, - ResourceDeleteEmailRedactor::TEMPLATE, - ], - 'user', - /** - * @param \Cake\ORM\Entity[] $emailQueueEntities - */ - function (array $emailQueueEntities, int $emailCount) { - $emailData = $emailQueueEntities[0]; - - $emailDigest = new EmailDigest(); - - foreach ($emailQueueEntities as $emailQueueEntity) { - $emailDigest->addEmailData($emailQueueEntity); - } - - $locale = $emailData->get('template_vars')['locale']; - $subject = (new LocaleService())->translateString( - $locale, - function () { - return __('Multiple passwords have been changed in passbolt'); - } - ); - - return $emailDigest - ->setSubject($subject) - ->addLayoutVar(LocaleEmailQueueListener::VIEW_VAR_KEY, $locale) - ->setTemplate(static::RESOURCE_CHANGES_TEMPLATE) - ->setEmailRecipient($emailData->get('email')) - ->addTemplateVar('user', $emailData->get('template_vars')['body']['user']) - ->addTemplateVar('fullBaseUrl', Configure::read('App.fullBaseUrl')) - ->addTemplateVar('count', $emailCount); - } - ); - } - - /** - * Create a new digest for the resource share emails. - * The marshaller will create a digest only if there is minimum 2 emails. - * It will create another digest if there is more than 50 emails. - * - * @return \Passbolt\EmailDigest\Utility\Digest\Digest - */ - private function createResourceShareDigest(): Digest - { - return new Digest( - __('{0} shared several items with you', '{0}'), - [ - ShareEmailRedactor::TEMPLATE, - ], - 'owner', - /** - * @param \Cake\ORM\Entity[] $emailQueueEntities - */ - function (array $emailQueueEntities, int $emailCount) { - $emailData = $emailQueueEntities[0]; - - $emailDigest = new EmailDigest(); - - foreach ($emailQueueEntities as $emailQueueEntity) { - $emailDigest->addEmailData($emailQueueEntity); - } - - $locale = $emailData->get('template_vars')['locale']; - $subject = (new LocaleService())->translateString( - $locale, - function () { - return __('Multiple passwords have been shared with you in passbolt'); - } - ); - - return $emailDigest - ->setSubject($subject) - ->addLayoutVar(LocaleEmailQueueListener::VIEW_VAR_KEY, $locale) - ->setTemplate(static::RESOURCE_SHARE_MULTIPLE_TEMPLATE) - ->setEmailRecipient($emailData->get('email')) - ->addTemplateVar('owner', $emailData->get('template_vars')['body']['owner']) - ->addTemplateVar('fullBaseUrl', Configure::read('App.fullBaseUrl')) - ->addTemplateVar('count', $emailCount); - } - ); - } -} diff --git a/templates/email/html/LU/group_user_delete.php b/templates/email/html/LU/group_user_delete.php index 0fb5550766..156855637a 100644 --- a/templates/email/html/LU/group_user_delete.php +++ b/templates/email/html/LU/group_user_delete.php @@ -27,7 +27,7 @@ 'text' => $this->element('Email/module/avatar_text', [ 'user' => $admin, 'datetime' => FrozenTime::now(), - 'text' => __('{0} removed you from the group {1}', Purifier::clean($admin['profile']['first_name']), Purifier::clean($group['name'])) + 'text' => $title, ]) ]); diff --git a/templates/email/html/LU/group_users_change.php b/templates/email/html/LU/group_users_change.php index 2e96f5c0af..2b877c9d13 100644 --- a/templates/email/html/LU/group_users_change.php +++ b/templates/email/html/LU/group_users_change.php @@ -21,13 +21,14 @@ } $user = $body['user']; $count = $body['count']; +$subject = $body['subject']; echo $this->element('Email/module/avatar',[ 'url' => AvatarHelper::getAvatarUrl($user['profile']['avatar']), 'text' => $this->element('Email/module/avatar_text', [ 'user' => $user, 'datetime' => new FrozenTime(), - 'text' => __('Edited your membership in several groups') + 'text' => $subject, ]) ]); diff --git a/templates/email/html/LU/groups_delete.php b/templates/email/html/LU/groups_delete.php index c9e9f176d3..f0ed0b1aee 100644 --- a/templates/email/html/LU/groups_delete.php +++ b/templates/email/html/LU/groups_delete.php @@ -14,20 +14,21 @@ */ use App\Utility\Purifier; use App\View\Helper\AvatarHelper; +use Cake\I18n\FrozenTime; use Cake\Routing\Router; if (PHP_SAPI === 'cli') { Router::fullBaseUrl($body['fullBaseUrl']); } +$subject = $body['subject']; $admin = $body['admin']; -$group = $body['group']; $count = $body['count']; echo $this->element('Email/module/avatar',[ 'url' => AvatarHelper::getAvatarUrl($admin['profile']['avatar']), 'text' => $this->element('Email/module/avatar_text', [ 'user' => $admin, - 'datetime' => $group['modified'], - 'text' => __('{0} deleted several group', Purifier::clean($admin['profile']['first_name'])) + 'datetime' => new FrozenTime(), + 'text' => $subject, ]) ]); diff --git a/templates/email/html/LU/resource_delete.php b/templates/email/html/LU/resource_delete.php index c2119e7aff..5c4a934615 100644 --- a/templates/email/html/LU/resource_delete.php +++ b/templates/email/html/LU/resource_delete.php @@ -18,6 +18,7 @@ if (PHP_SAPI === 'cli') { Router::fullBaseUrl($body['fullBaseUrl']); } +$subject = $body['subject']; $user = $body['user']; $resource = $body['resource']; $showUsername = $body['showUsername']; @@ -29,7 +30,7 @@ 'text' => $this->element('Email/module/avatar_text', [ 'user' => $user, 'datetime' => $resource['modified'], - 'text' => __('{0} deleted the password {1}', Purifier::clean($user['profile']['first_name']), Purifier::clean($resource['name'])) + 'text' => $subject, ]) ]); diff --git a/templates/email/html/LU/resources_change.php b/templates/email/html/LU/resources_change.php index e797fd05b9..c1e6feb14a 100644 --- a/templates/email/html/LU/resources_change.php +++ b/templates/email/html/LU/resources_change.php @@ -21,13 +21,14 @@ } $user = $body['user']; $count = $body['count']; +$subject = $body['subject']; echo $this->element('Email/module/avatar',[ 'url' => AvatarHelper::getAvatarUrl($user['profile']['avatar']), 'text' => $this->element('Email/module/avatar_text', [ 'user' => $user, 'datetime' => new FrozenTime(), - 'text' => __('Edited multiple resources') + 'text' => $subject, ]) ]); diff --git a/tests/Factory/UserFactory.php b/tests/Factory/UserFactory.php index aa4c1f90e1..90855bf1b8 100644 --- a/tests/Factory/UserFactory.php +++ b/tests/Factory/UserFactory.php @@ -230,4 +230,14 @@ public function withAvatar(string $filename = FIXTURES . 'Avatar' . DS . 'ada.jp 'data' => file_get_contents($filename), ]); } + + /** + * @return UserFactory this + */ + public function withAvatarNull(): self + { + return $this->with('Profiles.Avatars', [ + 'data' => null, + ]); + } } diff --git a/tests/Lib/AppIntegrationTestCase.php b/tests/Lib/AppIntegrationTestCase.php index 49252639ba..7125753068 100644 --- a/tests/Lib/AppIntegrationTestCase.php +++ b/tests/Lib/AppIntegrationTestCase.php @@ -47,8 +47,7 @@ use Cake\TestSuite\TestCase; use CakephpFixtureFactories\Scenario\ScenarioAwareTrait; use CakephpTestSuiteLight\Fixture\TruncateDirtyTables; -use Passbolt\EmailDigest\Utility\Digest\DigestsPool; -use Passbolt\EmailDigest\Utility\Factory\DigestFactory; +use Passbolt\EmailDigest\Utility\Digest\DigestTemplateRegistry; use Passbolt\EmailNotificationSettings\Utility\EmailNotificationSettings; abstract class AppIntegrationTestCase extends TestCase @@ -102,8 +101,7 @@ public function tearDown(): void { OpenPGPBackendFactory::reset(); UserAction::destroy(); - DigestsPool::clearInstance(); - DigestFactory::clearInstance(); + DigestTemplateRegistry::clearInstance(); EmailNotificationSettings::flushCache(); $this->clearPlugins(); parent::tearDown(); diff --git a/tests/Lib/AppTestCase.php b/tests/Lib/AppTestCase.php index 126d447dec..5a1178306d 100644 --- a/tests/Lib/AppTestCase.php +++ b/tests/Lib/AppTestCase.php @@ -35,8 +35,7 @@ use Cake\TestSuite\TestCase; use CakephpFixtureFactories\ORM\FactoryTableRegistry; use CakephpTestSuiteLight\Fixture\TruncateDirtyTables; -use Passbolt\EmailDigest\Utility\Digest\DigestsPool; -use Passbolt\EmailDigest\Utility\Factory\DigestFactory; +use Passbolt\EmailDigest\Utility\Digest\DigestTemplateRegistry; use Passbolt\EmailNotificationSettings\Utility\EmailNotificationSettings; abstract class AppTestCase extends TestCase @@ -77,8 +76,7 @@ public function setUp(): void */ public function tearDown(): void { - DigestsPool::clearInstance(); - DigestFactory::clearInstance(); + DigestTemplateRegistry::clearInstance(); EmailNotificationSettings::flushCache(); $this->clearPlugins(); FactoryTableRegistry::getTableLocator()->clear(); diff --git a/tests/Lib/SolutionBootstrapperTestCase.php b/tests/Lib/SolutionBootstrapperTestCase.php index 635b271320..57757e63df 100644 --- a/tests/Lib/SolutionBootstrapperTestCase.php +++ b/tests/Lib/SolutionBootstrapperTestCase.php @@ -21,6 +21,8 @@ use Cake\TestSuite\IntegrationTestTrait; use Cake\TestSuite\TestCase; use Cake\Utility\Hash; +use Passbolt\EmailDigest\Utility\Digest\DigestTemplateRegistry; +use Passbolt\EmailNotificationSettings\Utility\EmailNotificationSettings; abstract class SolutionBootstrapperTestCase extends TestCase { @@ -37,6 +39,8 @@ public function setUp(): void parent::setUp(); $this->app = $this->createApp(); $this->clearPlugins(); + DigestTemplateRegistry::clearInstance(); + EmailNotificationSettings::flushCache(); } public function tearDown(): void diff --git a/tests/Lib/Utility/EmailTestTrait.php b/tests/Lib/Utility/EmailTestTrait.php index 3c8d17ee26..a86497351c 100644 --- a/tests/Lib/Utility/EmailTestTrait.php +++ b/tests/Lib/Utility/EmailTestTrait.php @@ -120,7 +120,7 @@ public function assertMailSentToAt(int $at, array $recipient, string $message = */ public function assertMailContainsAt(int $at, string $contents, string $message = ''): void { - $this->assertTextContains($contents, $this->getMailAt($at)->getBodyString(), $message); + $this->assertTextContains(h($contents), $this->getMailAt($at)->getBodyString(), $message); } /** @@ -132,7 +132,7 @@ public function assertMailContainsAt(int $at, string $contents, string $message */ public function assertMailSubjectContainsAt(int $at, string $contents, string $message = ''): void { - $this->assertTextContains($contents, $this->getMailAt($at)->getOriginalSubject(), $message); + $this->assertTextContains(h($contents), $this->getMailAt($at)->getOriginalSubject(), $message); } /** From 0c60ad427d3292b6e70fb3030a10b2e9a3ecd298 Mon Sep 17 00:00:00 2001 From: jpramirez Date: Fri, 17 Nov 2023 17:11:32 +0000 Subject: [PATCH 08/16] Disable hydration on ResourcesIndexController --- .../Model/Behavior/FolderizableBehavior.php | 44 ++++++++++------- .../src/Model/Table/SecretAccessesTable.php | 26 +++++++--- .../LogResourcesIndexControllerTest.php | 49 +++++++++++++++++++ .../Component/ApiPaginationComponent.php | 13 +---- .../Resources/ResourcesIndexController.php | 27 +++++++--- .../Resources/ResourcesViewController.php | 2 +- .../Secrets/SecretsViewController.php | 2 +- src/Model/Entity/Avatar.php | 26 ---------- src/Model/Event/TableFindIndexBefore.php | 4 +- src/Model/Table/AvatarsTable.php | 28 ++++++++--- .../Resources/ResourcesFindersTrait.php | 1 - tests/Lib/Model/EmailQueueTrait.php | 2 + .../Avatars/AvatarsViewControllerTest.php | 14 ------ .../Table/Avatars/AddContainAvatarTest.php | 32 +++++++++--- .../FindViewAcoPermissionsTest.php | 10 ++-- 15 files changed, 174 insertions(+), 106 deletions(-) create mode 100644 plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php diff --git a/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php b/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php index 683d6cdb06..298d96d9e5 100644 --- a/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php +++ b/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php @@ -114,7 +114,7 @@ public function initialize(array $config): void * @return \Cake\ORM\Query * @see $_defaultConfig */ - public function findFolderParentId(Query $query, array $options) + public function findFolderParentId(Query $query, array $options): Query { return $this->formatResults($query, $options['user_id']); } @@ -126,20 +126,22 @@ public function findFolderParentId(Query $query, array $options) * @param string $userId The user id for whom the request has been executed. * @return \Cake\ORM\Query */ - public function formatResults(Query $query, string $userId) + public function formatResults(Query $query, string $userId): Query { return $query->formatResults(function (CollectionInterface $results) use ($userId) { $itemsIds = $results->extract('id')->toArray(); $itemsFolderParentIdsHash = $this->getItemsFolderParentIdHash($itemsIds, $userId); $itemsUsageHash = $this->getItemsUsageHash($itemsIds); - return $results->map(function (EntityInterface $entity) use ($itemsFolderParentIdsHash, $itemsUsageHash) { - if (array_key_exists($entity->get('id'), $itemsFolderParentIdsHash)) { - $folderParentId = $itemsFolderParentIdsHash[$entity->get('id')]; + // The entity passed may be an entity interface, or an array if the hydration has been disabled + return $results->map(function ($entity) use ($itemsFolderParentIdsHash, $itemsUsageHash) { + $entityId = $entity['id']; + if (array_key_exists($entityId, $itemsFolderParentIdsHash)) { + $folderParentId = $itemsFolderParentIdsHash[$entityId]; $entity = $this->addFolderParentIdProperty($entity, $folderParentId); } - if (array_key_exists($entity->get('id'), $itemsUsageHash)) { - $isPersonal = $itemsUsageHash[$entity->get('id')] === 1; + if (array_key_exists($entityId, $itemsUsageHash)) { + $isPersonal = $itemsUsageHash[$entityId] === 1; $entity = $this->addPersonalStatusProperty($entity, $isPersonal); } @@ -214,14 +216,18 @@ private function getItemsUsageHash(array $itemsIds) /** * Add the folder_parent_id property to an entity * - * @param \Cake\Datasource\EntityInterface $entity The target entity + * @param array|\Cake\Datasource\EntityInterface $entity The target entity * @param string|null $folderParentId The folder parent id - * @return \Cake\Datasource\EntityInterface + * @return array|\Cake\Datasource\EntityInterface */ - private function addFolderParentIdProperty(EntityInterface $entity, ?string $folderParentId = null) + private function addFolderParentIdProperty($entity, ?string $folderParentId = null) { - $entity->setVirtual([self::FOLDER_PARENT_ID_PROPERTY], true); - $entity->set(self::FOLDER_PARENT_ID_PROPERTY, $folderParentId); + if ($entity instanceof EntityInterface) { + $entity->setVirtual([self::FOLDER_PARENT_ID_PROPERTY], true); + $entity->set(self::FOLDER_PARENT_ID_PROPERTY, $folderParentId); + } else { + $entity[self::FOLDER_PARENT_ID_PROPERTY] = $folderParentId; + } return $entity; } @@ -229,14 +235,18 @@ private function addFolderParentIdProperty(EntityInterface $entity, ?string $fol /** * Add the personal status property to an entity * - * @param \Cake\Datasource\EntityInterface $entity The target entity + * @param array|\Cake\Datasource\EntityInterface $entity The target entity * @param bool $isPersonal The status - * @return \Cake\Datasource\EntityInterface + * @return array|\Cake\Datasource\EntityInterface */ - private function addPersonalStatusProperty(EntityInterface $entity, bool $isPersonal) + private function addPersonalStatusProperty($entity, bool $isPersonal) { - $entity->setVirtual([self::PERSONAL_PROPERTY], true); - $entity->set(self::PERSONAL_PROPERTY, $isPersonal); + if ($entity instanceof EntityInterface) { + $entity->setVirtual([self::PERSONAL_PROPERTY], true); + $entity->set(self::PERSONAL_PROPERTY, $isPersonal); + } else { + $entity[self::PERSONAL_PROPERTY] = $isPersonal; + } return $entity; } diff --git a/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php b/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php index 8b1a9c4a9a..9f78dabbe8 100644 --- a/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php +++ b/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php @@ -131,20 +131,32 @@ public function buildEntity(array $data): SecretAccess } /** - * Create a new SecretAccess + * Create a new SecretAccess from a secret entity * - * @param \App\Model\Entity\Secret $secret the secret entity * @param \App\Utility\UserAccessControl $uac user access control object + * @param \App\Model\Entity\Secret $secret the secret entity * @return bool|\Cake\Datasource\EntityInterface|false|mixed */ - public function create(Secret $secret, UserAccessControl $uac) + public function createFromSecretEntity(UserAccessControl $uac, Secret $secret) { - $userId = $uac->getId(); + return $this->createFromSecretDetails($uac, $secret->resource_id, $secret->id); + } + /** + * Create a new SecretAccess from a secret entity + * + * @param \App\Utility\UserAccessControl $uac user access control object + * @param string $resourceId The resource identifier + * @param string $secretId The secret identifier + * @return bool|\Cake\Datasource\EntityInterface|false|mixed + * @throws \App\Error\Exception\ValidationException + */ + public function createFromSecretDetails(UserAccessControl $uac, string $resourceId, string $secretId) + { $data = [ - 'user_id' => $userId, - 'resource_id' => $secret->resource_id, - 'secret_id' => $secret->id, + 'user_id' => $uac->getId(), + 'resource_id' => $resourceId, + 'secret_id' => $secretId, ]; // Check validation rules. diff --git a/plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php b/plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php new file mode 100644 index 0000000000..39b74272d6 --- /dev/null +++ b/plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php @@ -0,0 +1,49 @@ +logInAsUser(); + $otherUser = UserFactory::make()->persist(); + /** @var \App\Model\Entity\Resource $resource */ + $resource = ResourceFactory::make() + ->withPermissionsFor([$user]) + ->withSecretsFor([$user, $otherUser]) + ->persist(); + + $urlParameter = 'contain[secret]=1'; + $this->getJson("/resources.json?$urlParameter"); + $this->assertSuccess(); + + $conditions = [ + 'user_id' => $user->id, + 'resource_id' => $resource->id, + 'secret_id' => $resource->secrets[0]->id, + ]; + $this->assertSame(1, SecretAccessFactory::find()->where([$conditions])->count()); + $this->assertSame(1, SecretAccessFactory::count()); + } +} diff --git a/src/Controller/Component/ApiPaginationComponent.php b/src/Controller/Component/ApiPaginationComponent.php index 95d958340b..9fceb07884 100644 --- a/src/Controller/Component/ApiPaginationComponent.php +++ b/src/Controller/Component/ApiPaginationComponent.php @@ -22,17 +22,11 @@ use Cake\Http\ServerRequest; use Cake\Utility\Inflector; -/** - * @property \Authentication\Controller\Component\AuthenticationComponent $Authentication - */ class ApiPaginationComponent extends BaseApiComponent { public const MAX_LIMIT = 1000000; - /** - * @var array - */ - public $defaultConfig = [ + public array $defaultConfig = [ 'visible' => [ 'count', 'limit', @@ -40,10 +34,7 @@ class ApiPaginationComponent extends BaseApiComponent ], ]; - /** - * @var array - */ - public $paginate; + public array $paginate; /** * @inheritDoc diff --git a/src/Controller/Resources/ResourcesIndexController.php b/src/Controller/Resources/ResourcesIndexController.php index 2fbebfb15a..842494a1c2 100644 --- a/src/Controller/Resources/ResourcesIndexController.php +++ b/src/Controller/Resources/ResourcesIndexController.php @@ -20,6 +20,8 @@ use App\Controller\AppController; use Cake\Core\Configure; use Cake\Http\Exception\InternalErrorException; +use Cake\ORM\Query; +use Cake\Utility\Hash; /** * @property \BryanCrowe\ApiPagination\Controller\Component\ApiPaginationComponent $ApiPagination @@ -83,32 +85,43 @@ public function index() $options = $this->QueryString->get($whitelist); // Retrieve the resources. - $resources = $this->Resources->findIndex($this->User->id(), $options); - $this->_logSecretAccesses($resources->all()->toArray()); + $resources = $this->Resources->findIndex($this->User->id(), $options)->disableHydration(); $this->paginate($resources); + $this->_logSecretAccesses($resources, $options); $this->success(__('The operation was successful.'), $resources); } /** * Log secrets accesses in secretAccesses table. * - * @param array $resources resources + * @param \Cake\ORM\Query $resources resources + * @param array $queryOptions The query options * @return void */ - protected function _logSecretAccesses(array $resources) + protected function _logSecretAccesses(Query $resources, array $queryOptions) { + $containSecret = (bool)Hash::get($queryOptions, 'contain.secret'); + if (!$containSecret) { + return; + } + if (!$this->Resources->getAssociation('Secrets')->hasAssociation('SecretAccesses')) { return; } foreach ($resources as $resource) { - if (!isset($resource->secrets)) { + $secrets = Hash::get($resource, 'secrets'); + if (!isset($secrets)) { continue; } - foreach ($resource->secrets as $secret) { + foreach ($secrets as $secret) { try { - $this->Resources->Secrets->SecretAccesses->create($secret, $this->User->getAccessControl()); + $this->Resources->Secrets->SecretAccesses->createFromSecretDetails( + $this->User->getAccessControl(), + Hash::get($secret, 'resource_id'), + Hash::get($secret, 'id'), + ); } catch (\Exception $e) { throw new InternalErrorException('Could not log secret access entry.', 500, $e); } diff --git a/src/Controller/Resources/ResourcesViewController.php b/src/Controller/Resources/ResourcesViewController.php index 57d73ee963..5881d762cb 100644 --- a/src/Controller/Resources/ResourcesViewController.php +++ b/src/Controller/Resources/ResourcesViewController.php @@ -98,7 +98,7 @@ protected function _logSecretAccesses(Resource $resource): void try { /** @var \Passbolt\Log\Model\Table\SecretAccessesTable $SecretAccesses */ $SecretAccesses = $Secrets->getAssociation('SecretAccesses'); - $SecretAccesses->create($secret, $this->User->getAccessControl()); + $SecretAccesses->createFromSecretEntity($this->User->getAccessControl(), $secret); } catch (Exception $e) { throw new InternalErrorException('Could not log secret access entry.', 500, $e); } diff --git a/src/Controller/Secrets/SecretsViewController.php b/src/Controller/Secrets/SecretsViewController.php index 3834bdbe3f..70a3c36433 100644 --- a/src/Controller/Secrets/SecretsViewController.php +++ b/src/Controller/Secrets/SecretsViewController.php @@ -84,7 +84,7 @@ protected function _logSecretAccesses(Secret $secret, UserAccessControl $uac) { try { if ($this->Secrets->hasAssociation('SecretAccesses')) { - $this->Secrets->SecretAccesses->create($secret, $uac); + $this->Secrets->SecretAccesses->createFromSecretEntity($uac, $secret); } } catch (Exception $e) { throw new InternalErrorException('Could not log secret access entry.', 500, $e); diff --git a/src/Model/Entity/Avatar.php b/src/Model/Entity/Avatar.php index d9b18a03ed..0474ae591c 100644 --- a/src/Model/Entity/Avatar.php +++ b/src/Model/Entity/Avatar.php @@ -16,8 +16,6 @@ */ namespace App\Model\Entity; -use App\View\Helper\AvatarHelper; -use Cake\Core\Configure; use Cake\ORM\Entity; use Psr\Http\Message\StreamInterface; @@ -28,12 +26,9 @@ * @property \Cake\I18n\FrozenTime $created_at * @property \Cake\I18n\FrozenTime|null $updated_at * @property \App\Model\Entity\Profile $profile - * @property-read array $url */ class Avatar extends Entity { - protected $_virtual = ['url']; - /** * The avatar data never needs to be served. it is stored in cache. * @@ -57,27 +52,6 @@ class Avatar extends Entity '*' => true, ]; - /** - * Url virtual field implementation. - * - * @return array - */ - protected function _getUrl() - { - $sizes = Configure::read('FileStorage.imageSizes.Avatar'); - $avatarsPath = []; - // Add path for each available size. - foreach ($sizes as $size => $filters) { - $avatarsPath[$size] = AvatarHelper::getAvatarUrl([ - 'id' => $this->id, - 'data' => $this->data, - ], $size); - } - - // Transform original model to add paths. - return $avatarsPath; - } - /** * Get data in string format. * diff --git a/src/Model/Event/TableFindIndexBefore.php b/src/Model/Event/TableFindIndexBefore.php index 26274ea13c..d81e2a858a 100644 --- a/src/Model/Event/TableFindIndexBefore.php +++ b/src/Model/Event/TableFindIndexBefore.php @@ -92,9 +92,9 @@ private function setQuery(Query $query) * @param \Cake\ORM\Query $query Query * @param \App\Model\Table\Dto\FindIndexOptions $options Options * @param \Cake\ORM\Table $table Table - * @return \App\Model\Event\TableFindIndexBefore + * @return self */ - public static function create(Query $query, FindIndexOptions $options, Table $table) + public static function create(Query $query, FindIndexOptions $options, Table $table): self { return new static(static::EVENT_NAME, $table, [ 'query' => $query, diff --git a/src/Model/Table/AvatarsTable.php b/src/Model/Table/AvatarsTable.php index a693c8a095..d9b94b9361 100644 --- a/src/Model/Table/AvatarsTable.php +++ b/src/Model/Table/AvatarsTable.php @@ -193,16 +193,32 @@ public function afterDelete(Event $event, Avatar $avatar, \ArrayObject $options) * Used mainly to populate an avatar when no there is no result with the default avatar url. * * @param \Cake\Collection\CollectionInterface $avatars list of avatars (\App\Model\Entity\Avatar) + * @param bool $isHydrationEnabled if hydration is enabled, return an Avatar object, otherwise an array * @return mixed - * @deprecated the fallback avatar url is handled by the AvatarHelper. */ - public static function formatResults(CollectionInterface $avatars) + private static function formatResults(CollectionInterface $avatars, bool $isHydrationEnabled) { - return $avatars->map(function ($avatar) { + return $avatars->map(function ($avatar) use ($isHydrationEnabled) { + $sizes = Configure::read('FileStorage.imageSizes.Avatar'); + $url = []; + // Add path for each available size. + foreach ($sizes as $size => $filters) { + $url[$size] = AvatarHelper::getAvatarUrl([ + 'id' => $avatar['id'] ?? null, + ], $size); + } + if (empty($avatar)) { // If avatar is empty, we instantiate one. // The virtual field will take care of retrieving the default avatar. - $avatar = new Avatar(); + $avatar = $isHydrationEnabled ? new Avatar() : []; + } + + if ($avatar instanceof Avatar) { + $avatar->setVirtual(['url'], true); + $avatar->set('url', (object)$url); + } else { + $avatar['url'] = $url; } return $avatar; @@ -222,8 +238,8 @@ public static function addContainAvatar(): array // Formatter for empty avatars. return $q ->select(['Avatars.id', 'Avatars.profile_id', 'Avatars.created', 'Avatars.modified']) - ->formatResults(function (CollectionInterface $avatars) { - return AvatarsTable::formatResults($avatars); + ->formatResults(function (CollectionInterface $avatars, Query $mainQuery) { + return AvatarsTable::formatResults($avatars, $mainQuery->isHydrationEnabled()); }); }, ]; diff --git a/src/Model/Traits/Resources/ResourcesFindersTrait.php b/src/Model/Traits/Resources/ResourcesFindersTrait.php index 01d924b680..f9c4e218a0 100644 --- a/src/Model/Traits/Resources/ResourcesFindersTrait.php +++ b/src/Model/Traits/Resources/ResourcesFindersTrait.php @@ -55,7 +55,6 @@ public function findIndex(string $userId, ?array $options = []) $event = TableFindIndexBefore::create($query, $findIndexOptions, $this); - /** @var \App\Model\Event\TableFindIndexBefore $event */ $this->getEventManager()->dispatch($event); // Filter out deleted resources diff --git a/tests/Lib/Model/EmailQueueTrait.php b/tests/Lib/Model/EmailQueueTrait.php index 0b8102a438..945388d958 100644 --- a/tests/Lib/Model/EmailQueueTrait.php +++ b/tests/Lib/Model/EmailQueueTrait.php @@ -153,6 +153,7 @@ protected function assertEmailInBatchContains( ): void { if ($htmlSpecialChar) { $string = h($string); + $string = htmlspecialchars($string); } $this->assertStringContainsString($string, $this->renderEmail($i), $message); } @@ -173,6 +174,7 @@ protected function assertEmailInBatchNotContains( ): void { if ($htmlSpecialChar) { $string = h($string); + $string = htmlspecialchars($string); } $this->assertStringNotContainsString($string, $this->renderEmail($i), $message); } diff --git a/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php b/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php index 3e5fa414a0..4730dc5c2b 100644 --- a/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php +++ b/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php @@ -111,13 +111,6 @@ public function testViewAvatarsViewController_ViewExistentAvatar(string $format) $this->get('avatars/view/' . $avatar->id . '/' . $format . AvatarHelper::IMAGE_EXTENSION); $this->assertResponseEquals($expectedFileContent); - - // Ensure that the virtual field is correctly constructed. - $virtualField = [ - AvatarsConfigurationService::FORMAT_MEDIUM => AvatarHelper::getAvatarUrl($avatar->toArray(), AvatarsConfigurationService::FORMAT_MEDIUM), - AvatarsConfigurationService::FORMAT_SMALL => AvatarHelper::getAvatarUrl($avatar->toArray()), - ]; - $this->assertSame($virtualField, $avatar->url); } /** @@ -142,13 +135,6 @@ public function testAvatarsViewController_ViewExistentAvatarWithDeletedFile(stri $this->get('avatars/view/' . $avatar->id . '/' . $format . AvatarHelper::IMAGE_EXTENSION); $this->assertResponseEquals($expectedFileContent); - - // Ensure that the virtual field is correctly constructed. - $virtualField = [ - AvatarsConfigurationService::FORMAT_MEDIUM => AvatarHelper::getAvatarUrl($avatar->toArray(), AvatarsConfigurationService::FORMAT_MEDIUM), - AvatarsConfigurationService::FORMAT_SMALL => AvatarHelper::getAvatarUrl($avatar->toArray()), - ]; - $this->assertSame($virtualField, $avatar->url); } /** diff --git a/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php b/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php index e76abb1b15..6fd6563c64 100644 --- a/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php +++ b/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php @@ -35,9 +35,18 @@ public function setUp(): void { parent::setUp(); $this->Users = $this->fetchTable('Users'); + $this->loadRoutes(); } - public function testAvatarsTableAddContainAvatar_Should_Not_Retrieve_Avatar_Data() + public function enableHydration(): array + { + return [[true], [false]]; + } + + /** + * @dataProvider enableHydration + */ + public function testAvatarsTableAddContainAvatar_Should_Not_Retrieve_Avatar_Data(bool $isHydrationEnabled) { $user = UserFactory::make()->withAvatar()->user()->persist(); @@ -47,14 +56,18 @@ public function testAvatarsTableAddContainAvatar_Should_Not_Retrieve_Avatar_Data ->contain([ 'Profiles' => AvatarsTable::addContainAvatar(), ]) + ->enableHydration($isHydrationEnabled) ->firstOrFail(); - $this->assertSame($user->profile->avatar->id, $retrievedUser->profile->avatar->id); + $this->assertSame($user->profile->avatar->id, $retrievedUser['profile']['avatar']['id']); $this->assertNotNull($user->profile->avatar->data); - $this->assertNull($retrievedUser->profile->avatar->data ?? null); + $this->assertNull($retrievedUser['profile']['avatar']['data'] ?? null); } - public function testAvatarsTableAddContainAvatar_On_Empty_Avatar() + /** + * @dataProvider enableHydration + */ + public function testAvatarsTableAddContainAvatar_On_Empty_Avatar(bool $isHydrationEnabled) { $user = UserFactory::make()->user()->persist(); @@ -64,12 +77,17 @@ public function testAvatarsTableAddContainAvatar_On_Empty_Avatar() ->contain([ 'Profiles' => AvatarsTable::addContainAvatar(), ]) + ->enableHydration($isHydrationEnabled) ->firstOrFail(); $this->assertNull($user->profile->avatar->id ?? null); $this->assertNull($user->profile->avatar->data ?? null); - $this->assertNull($retrievedUser->profile->avatar->id ?? null); - $this->assertNull($retrievedUser->profile->avatar->data ?? null); - $this->assertIsArray($retrievedUser->profile->avatar->url); + $this->assertNull($retrievedUser['profile']['avatar']['id'] ?? null); + $this->assertNull($retrievedUser['profile']['avatar']['data'] ?? null); + if ($isHydrationEnabled) { + $this->assertIsObject($retrievedUser['profile']['avatar']['url']); + } else { + $this->assertIsArray($retrievedUser['profile']['avatar']['url']); + } } } diff --git a/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php b/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php index f77a3ce571..12235959a7 100644 --- a/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php +++ b/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php @@ -19,6 +19,7 @@ use App\Model\Table\PermissionsTable; use App\Test\Lib\AppTestCase; +use App\Test\Lib\Model\AvatarsModelTrait; use App\Utility\UuidFactory; use Cake\ORM\TableRegistry; use Cake\Utility\Hash; @@ -26,6 +27,8 @@ class FindViewAcoPermissionsTest extends AppTestCase { + use AvatarsModelTrait; + public $fixtures = ['app.Base/Groups', 'app.Base/Permissions', 'app.Base/Profiles', 'app.Base/Resources', 'app.Base/Users']; /** @@ -81,12 +84,7 @@ public function testContainUserProfileAvatar() $permission = Hash::extract($permissions->toArray(), '{n}[aro=User]')[0]; $this->assertPermissionAttributes($permission); $this->assertProfileAttributes($permission->user->profile); - $this->assertObjectHasAttribute('avatar', $permission->user->profile); - $this->assertArrayHasKey('url', $permission->user->profile->avatar); - $this->assertArrayHasKey('medium', $permission->user->profile->avatar->url); - $this->assertArrayHasKey('small', $permission->user->profile->avatar->url); - $this->assertStringContainsString('/img/avatar/user_medium.png', $permission->user->profile->avatar->url['medium']); - $this->assertStringContainsString('/img/avatar/user.png', $permission->user->profile->avatar->url['small']); + $this->assertAvatarAttributes($permission->user->profile->avatar); } public function testPermissions() From 546add34c31897044fd589d02dfb395a814873aa Mon Sep 17 00:00:00 2001 From: Crowdin Date: Fri, 17 Nov 2023 17:37:38 +0000 Subject: [PATCH 09/16] New translations default.po (Korean) [skip-ci] --- resources/locales/ko_KR/default.po | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/resources/locales/ko_KR/default.po b/resources/locales/ko_KR/default.po index 8bac964b52..116ac2dbb1 100644 --- a/resources/locales/ko_KR/default.po +++ b/resources/locales/ko_KR/default.po @@ -2,7 +2,7 @@ msgid "" msgstr "" "Project-Id-Version: 41c2572bd9bd4cc908d3e09e0cbed6e5\n" "POT-Creation-Date: 2023-10-27 11:15+0000\n" -"PO-Revision-Date: 2023-11-02 02:13\n" +"PO-Revision-Date: 2023-11-17 17:37\n" "Last-Translator: NAME \n" "Language-Team: Korean\n" "MIME-Version: 1.0\n" @@ -1046,7 +1046,7 @@ msgid "{0} deleted user {1}" msgstr "{0} 에 의해 {1} 사용자가 삭제됨" msgid "{0} has been suspended" -msgstr "{0} 이(가) 정지되었습니다." +msgstr "{0} 이(가) 중지되었습니다." msgid "Welcome to passbolt, {0}!" msgstr "패스볼트에 어서 오세요, {0}!" @@ -3506,10 +3506,10 @@ msgid "Account suspended" msgstr "계정 정지" msgid "Your account has been suspended." -msgstr "계정이 정지되었습니다." +msgstr "계정이 중지되었습니다." msgid "You are not able to sign in to passbolt and receive email notifications." -msgstr "패스볼트 로그인 할 수 없으며 이메일 알림을 받을 수 없습니다." +msgstr "패스볼트에 로그인 할 수 없으며 이메일 알림을 받을 수 없습니다." msgid "Other users can still share resources with you and add you to a group." msgstr "다른 사용자는 여전히 사용자와 리소스를 공유하고 사용자를 그룹에 추가할 수 있습니다." @@ -3554,7 +3554,7 @@ msgid "The user {0} has been suspended." msgstr "{0} 사용자가 중지됨." msgid "This user will not be able to sign in to passbolt and receive email notifications." -msgstr "이 사용자는 패스볼트 로그인 할 수 없으며 이메일 알림을 받을 수 없습니다." +msgstr "이 사용자는 패스볼트에 로그인을 할 수 없으며 이메일 알림도 받을 수 없습니다." msgid "Other users can share resources and add this user to a group." msgstr "다른 사용자는 리소스를 공유하고 이 사용자를 그룹에 추가할 수 있습니다" From d908e27a5488357eb77e1226376a0d3c73963cd0 Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Fri, 17 Nov 2023 19:03:45 +0100 Subject: [PATCH 10/16] PB-28730 v4.4.1-test.1 --- CHANGELOG.md | 11 +++++++++++ config/version.php | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c3358c626..381058a76f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [4.4.1-test.1] - 2023-11-17 +### Improved +- PB-27616 As a user I should see improved performances when retrieving resources on the GET resources.json entry point + +### Fixed +- PB-27957 As an administrator I should be notified that an administrator was deleted only when an administrator has been deleted, and not a regular user + +### Maintenance +- PB-27927 The unused user_agents table is now removed from the database +- PB-28616 The email digest plugin code was deeply refactored for enhanced usage and maintainability + ## [4.4.0] - 2023-11-07 ### Added - PB-27773 As an administrator I can deny access to the mobile setup screen with RBAC diff --git a/config/version.php b/config/version.php index 30a8b61739..6200149490 100644 --- a/config/version.php +++ b/config/version.php @@ -1,8 +1,8 @@ [ - 'version' => '4.4.0', - 'name' => 'Zombie', + 'version' => '4.4.1-test.1', + 'name' => 'TBD', ], 'php' => [ 'minVersion' => '7.4', From a552fb3165c5c6760b87a121c5c5904845ee32cb Mon Sep 17 00:00:00 2001 From: Ishan Vyas Date: Wed, 8 Nov 2023 17:18:31 +0530 Subject: [PATCH 11/16] PB-28521: Add migration to alter gpgkeys.uid column length to 769 Fixes LDAP sync issue when users have long names, they can be synced but they won't be able to finish their setup --- .../20231108114414_V450AlterUidOnGpgkeys.php | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php diff --git a/config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php b/config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php new file mode 100644 index 0000000000..1344d2c301 --- /dev/null +++ b/config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php @@ -0,0 +1,37 @@ +table('gpgkeys'); + $table + ->changeColumn('uid', 'string', ['limit' => 769]) + ->update(); + } +} From 258317c39d121788343efa9190f1167e6b72b964 Mon Sep 17 00:00:00 2001 From: Ishan Vyas Date: Mon, 20 Nov 2023 13:11:33 +0530 Subject: [PATCH 12/16] PB-28521: Rename migration to related it to release v4.4.1 --- ...OnGpgkeys.php => 20231108114414_V441AlterUidOnGpgkeys.php} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename config/Migrations/{20231108114414_V450AlterUidOnGpgkeys.php => 20231108114414_V441AlterUidOnGpgkeys.php} (92%) diff --git a/config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php b/config/Migrations/20231108114414_V441AlterUidOnGpgkeys.php similarity index 92% rename from config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php rename to config/Migrations/20231108114414_V441AlterUidOnGpgkeys.php index 1344d2c301..eae86080f4 100644 --- a/config/Migrations/20231108114414_V450AlterUidOnGpgkeys.php +++ b/config/Migrations/20231108114414_V441AlterUidOnGpgkeys.php @@ -12,12 +12,12 @@ * @copyright Copyright (c) Passbolt SA (https://www.passbolt.com) * @license https://opensource.org/licenses/AGPL-3.0 AGPL License * @link https://www.passbolt.com Passbolt(tm) - * @since 4.5.0 + * @since 4.4.1 */ use Migrations\AbstractMigration; -class V450AlterUidOnGpgkeys extends AbstractMigration +class V441AlterUidOnGpgkeys extends AbstractMigration { /** * Change Method. From e2df262c3c78ea540b78a0cdfb34b185c4ecdcd1 Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Mon, 20 Nov 2023 09:08:06 +0100 Subject: [PATCH 13/16] PB-28730 v4.4.1-test.2 --- CHANGELOG.md | 4 ++++ config/version.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 381058a76f..c61499372e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [4.4.1-test.2] - 2023-11-20 +### Maintenance +- PB-28521 Alter the gpgkeys.uid column length to 769 + ## [4.4.1-test.1] - 2023-11-17 ### Improved - PB-27616 As a user I should see improved performances when retrieving resources on the GET resources.json entry point diff --git a/config/version.php b/config/version.php index 6200149490..1826c0ca0d 100644 --- a/config/version.php +++ b/config/version.php @@ -1,7 +1,7 @@ [ - 'version' => '4.4.1-test.1', + 'version' => '4.4.1-test.2', 'name' => 'TBD', ], 'php' => [ From bdfefff846e7b7e07319e8e84ed70462f5385efe Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Mon, 20 Nov 2023 13:20:26 +0100 Subject: [PATCH 14/16] Revert "Disable hydration on ResourcesIndexController" This reverts commit 0c60ad427d3292b6e70fb3030a10b2e9a3ecd298. --- .../Model/Behavior/FolderizableBehavior.php | 44 +++++++---------- .../src/Model/Table/SecretAccessesTable.php | 26 +++------- .../LogResourcesIndexControllerTest.php | 49 ------------------- .../Component/ApiPaginationComponent.php | 13 ++++- .../Resources/ResourcesIndexController.php | 27 +++------- .../Resources/ResourcesViewController.php | 2 +- .../Secrets/SecretsViewController.php | 2 +- src/Model/Entity/Avatar.php | 26 ++++++++++ src/Model/Event/TableFindIndexBefore.php | 4 +- src/Model/Table/AvatarsTable.php | 28 +++-------- .../Resources/ResourcesFindersTrait.php | 1 + tests/Lib/Model/EmailQueueTrait.php | 2 - .../Avatars/AvatarsViewControllerTest.php | 14 ++++++ .../Table/Avatars/AddContainAvatarTest.php | 32 +++--------- .../FindViewAcoPermissionsTest.php | 10 ++-- 15 files changed, 106 insertions(+), 174 deletions(-) delete mode 100644 plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php diff --git a/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php b/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php index 298d96d9e5..683d6cdb06 100644 --- a/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php +++ b/plugins/PassboltCe/Folders/src/Model/Behavior/FolderizableBehavior.php @@ -114,7 +114,7 @@ public function initialize(array $config): void * @return \Cake\ORM\Query * @see $_defaultConfig */ - public function findFolderParentId(Query $query, array $options): Query + public function findFolderParentId(Query $query, array $options) { return $this->formatResults($query, $options['user_id']); } @@ -126,22 +126,20 @@ public function findFolderParentId(Query $query, array $options): Query * @param string $userId The user id for whom the request has been executed. * @return \Cake\ORM\Query */ - public function formatResults(Query $query, string $userId): Query + public function formatResults(Query $query, string $userId) { return $query->formatResults(function (CollectionInterface $results) use ($userId) { $itemsIds = $results->extract('id')->toArray(); $itemsFolderParentIdsHash = $this->getItemsFolderParentIdHash($itemsIds, $userId); $itemsUsageHash = $this->getItemsUsageHash($itemsIds); - // The entity passed may be an entity interface, or an array if the hydration has been disabled - return $results->map(function ($entity) use ($itemsFolderParentIdsHash, $itemsUsageHash) { - $entityId = $entity['id']; - if (array_key_exists($entityId, $itemsFolderParentIdsHash)) { - $folderParentId = $itemsFolderParentIdsHash[$entityId]; + return $results->map(function (EntityInterface $entity) use ($itemsFolderParentIdsHash, $itemsUsageHash) { + if (array_key_exists($entity->get('id'), $itemsFolderParentIdsHash)) { + $folderParentId = $itemsFolderParentIdsHash[$entity->get('id')]; $entity = $this->addFolderParentIdProperty($entity, $folderParentId); } - if (array_key_exists($entityId, $itemsUsageHash)) { - $isPersonal = $itemsUsageHash[$entityId] === 1; + if (array_key_exists($entity->get('id'), $itemsUsageHash)) { + $isPersonal = $itemsUsageHash[$entity->get('id')] === 1; $entity = $this->addPersonalStatusProperty($entity, $isPersonal); } @@ -216,18 +214,14 @@ private function getItemsUsageHash(array $itemsIds) /** * Add the folder_parent_id property to an entity * - * @param array|\Cake\Datasource\EntityInterface $entity The target entity + * @param \Cake\Datasource\EntityInterface $entity The target entity * @param string|null $folderParentId The folder parent id - * @return array|\Cake\Datasource\EntityInterface + * @return \Cake\Datasource\EntityInterface */ - private function addFolderParentIdProperty($entity, ?string $folderParentId = null) + private function addFolderParentIdProperty(EntityInterface $entity, ?string $folderParentId = null) { - if ($entity instanceof EntityInterface) { - $entity->setVirtual([self::FOLDER_PARENT_ID_PROPERTY], true); - $entity->set(self::FOLDER_PARENT_ID_PROPERTY, $folderParentId); - } else { - $entity[self::FOLDER_PARENT_ID_PROPERTY] = $folderParentId; - } + $entity->setVirtual([self::FOLDER_PARENT_ID_PROPERTY], true); + $entity->set(self::FOLDER_PARENT_ID_PROPERTY, $folderParentId); return $entity; } @@ -235,18 +229,14 @@ private function addFolderParentIdProperty($entity, ?string $folderParentId = nu /** * Add the personal status property to an entity * - * @param array|\Cake\Datasource\EntityInterface $entity The target entity + * @param \Cake\Datasource\EntityInterface $entity The target entity * @param bool $isPersonal The status - * @return array|\Cake\Datasource\EntityInterface + * @return \Cake\Datasource\EntityInterface */ - private function addPersonalStatusProperty($entity, bool $isPersonal) + private function addPersonalStatusProperty(EntityInterface $entity, bool $isPersonal) { - if ($entity instanceof EntityInterface) { - $entity->setVirtual([self::PERSONAL_PROPERTY], true); - $entity->set(self::PERSONAL_PROPERTY, $isPersonal); - } else { - $entity[self::PERSONAL_PROPERTY] = $isPersonal; - } + $entity->setVirtual([self::PERSONAL_PROPERTY], true); + $entity->set(self::PERSONAL_PROPERTY, $isPersonal); return $entity; } diff --git a/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php b/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php index 9f78dabbe8..8b1a9c4a9a 100644 --- a/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php +++ b/plugins/PassboltCe/Log/src/Model/Table/SecretAccessesTable.php @@ -131,32 +131,20 @@ public function buildEntity(array $data): SecretAccess } /** - * Create a new SecretAccess from a secret entity + * Create a new SecretAccess * - * @param \App\Utility\UserAccessControl $uac user access control object * @param \App\Model\Entity\Secret $secret the secret entity - * @return bool|\Cake\Datasource\EntityInterface|false|mixed - */ - public function createFromSecretEntity(UserAccessControl $uac, Secret $secret) - { - return $this->createFromSecretDetails($uac, $secret->resource_id, $secret->id); - } - - /** - * Create a new SecretAccess from a secret entity - * * @param \App\Utility\UserAccessControl $uac user access control object - * @param string $resourceId The resource identifier - * @param string $secretId The secret identifier * @return bool|\Cake\Datasource\EntityInterface|false|mixed - * @throws \App\Error\Exception\ValidationException */ - public function createFromSecretDetails(UserAccessControl $uac, string $resourceId, string $secretId) + public function create(Secret $secret, UserAccessControl $uac) { + $userId = $uac->getId(); + $data = [ - 'user_id' => $uac->getId(), - 'resource_id' => $resourceId, - 'secret_id' => $secretId, + 'user_id' => $userId, + 'resource_id' => $secret->resource_id, + 'secret_id' => $secret->id, ]; // Check validation rules. diff --git a/plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php b/plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php deleted file mode 100644 index 39b74272d6..0000000000 --- a/plugins/PassboltCe/Log/tests/TestCase/Controller/Resources/LogResourcesIndexControllerTest.php +++ /dev/null @@ -1,49 +0,0 @@ -logInAsUser(); - $otherUser = UserFactory::make()->persist(); - /** @var \App\Model\Entity\Resource $resource */ - $resource = ResourceFactory::make() - ->withPermissionsFor([$user]) - ->withSecretsFor([$user, $otherUser]) - ->persist(); - - $urlParameter = 'contain[secret]=1'; - $this->getJson("/resources.json?$urlParameter"); - $this->assertSuccess(); - - $conditions = [ - 'user_id' => $user->id, - 'resource_id' => $resource->id, - 'secret_id' => $resource->secrets[0]->id, - ]; - $this->assertSame(1, SecretAccessFactory::find()->where([$conditions])->count()); - $this->assertSame(1, SecretAccessFactory::count()); - } -} diff --git a/src/Controller/Component/ApiPaginationComponent.php b/src/Controller/Component/ApiPaginationComponent.php index 9fceb07884..95d958340b 100644 --- a/src/Controller/Component/ApiPaginationComponent.php +++ b/src/Controller/Component/ApiPaginationComponent.php @@ -22,11 +22,17 @@ use Cake\Http\ServerRequest; use Cake\Utility\Inflector; +/** + * @property \Authentication\Controller\Component\AuthenticationComponent $Authentication + */ class ApiPaginationComponent extends BaseApiComponent { public const MAX_LIMIT = 1000000; - public array $defaultConfig = [ + /** + * @var array + */ + public $defaultConfig = [ 'visible' => [ 'count', 'limit', @@ -34,7 +40,10 @@ class ApiPaginationComponent extends BaseApiComponent ], ]; - public array $paginate; + /** + * @var array + */ + public $paginate; /** * @inheritDoc diff --git a/src/Controller/Resources/ResourcesIndexController.php b/src/Controller/Resources/ResourcesIndexController.php index 842494a1c2..2fbebfb15a 100644 --- a/src/Controller/Resources/ResourcesIndexController.php +++ b/src/Controller/Resources/ResourcesIndexController.php @@ -20,8 +20,6 @@ use App\Controller\AppController; use Cake\Core\Configure; use Cake\Http\Exception\InternalErrorException; -use Cake\ORM\Query; -use Cake\Utility\Hash; /** * @property \BryanCrowe\ApiPagination\Controller\Component\ApiPaginationComponent $ApiPagination @@ -85,43 +83,32 @@ public function index() $options = $this->QueryString->get($whitelist); // Retrieve the resources. - $resources = $this->Resources->findIndex($this->User->id(), $options)->disableHydration(); + $resources = $this->Resources->findIndex($this->User->id(), $options); + $this->_logSecretAccesses($resources->all()->toArray()); $this->paginate($resources); - $this->_logSecretAccesses($resources, $options); $this->success(__('The operation was successful.'), $resources); } /** * Log secrets accesses in secretAccesses table. * - * @param \Cake\ORM\Query $resources resources - * @param array $queryOptions The query options + * @param array $resources resources * @return void */ - protected function _logSecretAccesses(Query $resources, array $queryOptions) + protected function _logSecretAccesses(array $resources) { - $containSecret = (bool)Hash::get($queryOptions, 'contain.secret'); - if (!$containSecret) { - return; - } - if (!$this->Resources->getAssociation('Secrets')->hasAssociation('SecretAccesses')) { return; } foreach ($resources as $resource) { - $secrets = Hash::get($resource, 'secrets'); - if (!isset($secrets)) { + if (!isset($resource->secrets)) { continue; } - foreach ($secrets as $secret) { + foreach ($resource->secrets as $secret) { try { - $this->Resources->Secrets->SecretAccesses->createFromSecretDetails( - $this->User->getAccessControl(), - Hash::get($secret, 'resource_id'), - Hash::get($secret, 'id'), - ); + $this->Resources->Secrets->SecretAccesses->create($secret, $this->User->getAccessControl()); } catch (\Exception $e) { throw new InternalErrorException('Could not log secret access entry.', 500, $e); } diff --git a/src/Controller/Resources/ResourcesViewController.php b/src/Controller/Resources/ResourcesViewController.php index 5881d762cb..57d73ee963 100644 --- a/src/Controller/Resources/ResourcesViewController.php +++ b/src/Controller/Resources/ResourcesViewController.php @@ -98,7 +98,7 @@ protected function _logSecretAccesses(Resource $resource): void try { /** @var \Passbolt\Log\Model\Table\SecretAccessesTable $SecretAccesses */ $SecretAccesses = $Secrets->getAssociation('SecretAccesses'); - $SecretAccesses->createFromSecretEntity($this->User->getAccessControl(), $secret); + $SecretAccesses->create($secret, $this->User->getAccessControl()); } catch (Exception $e) { throw new InternalErrorException('Could not log secret access entry.', 500, $e); } diff --git a/src/Controller/Secrets/SecretsViewController.php b/src/Controller/Secrets/SecretsViewController.php index 70a3c36433..3834bdbe3f 100644 --- a/src/Controller/Secrets/SecretsViewController.php +++ b/src/Controller/Secrets/SecretsViewController.php @@ -84,7 +84,7 @@ protected function _logSecretAccesses(Secret $secret, UserAccessControl $uac) { try { if ($this->Secrets->hasAssociation('SecretAccesses')) { - $this->Secrets->SecretAccesses->createFromSecretEntity($uac, $secret); + $this->Secrets->SecretAccesses->create($secret, $uac); } } catch (Exception $e) { throw new InternalErrorException('Could not log secret access entry.', 500, $e); diff --git a/src/Model/Entity/Avatar.php b/src/Model/Entity/Avatar.php index 0474ae591c..d9b18a03ed 100644 --- a/src/Model/Entity/Avatar.php +++ b/src/Model/Entity/Avatar.php @@ -16,6 +16,8 @@ */ namespace App\Model\Entity; +use App\View\Helper\AvatarHelper; +use Cake\Core\Configure; use Cake\ORM\Entity; use Psr\Http\Message\StreamInterface; @@ -26,9 +28,12 @@ * @property \Cake\I18n\FrozenTime $created_at * @property \Cake\I18n\FrozenTime|null $updated_at * @property \App\Model\Entity\Profile $profile + * @property-read array $url */ class Avatar extends Entity { + protected $_virtual = ['url']; + /** * The avatar data never needs to be served. it is stored in cache. * @@ -52,6 +57,27 @@ class Avatar extends Entity '*' => true, ]; + /** + * Url virtual field implementation. + * + * @return array + */ + protected function _getUrl() + { + $sizes = Configure::read('FileStorage.imageSizes.Avatar'); + $avatarsPath = []; + // Add path for each available size. + foreach ($sizes as $size => $filters) { + $avatarsPath[$size] = AvatarHelper::getAvatarUrl([ + 'id' => $this->id, + 'data' => $this->data, + ], $size); + } + + // Transform original model to add paths. + return $avatarsPath; + } + /** * Get data in string format. * diff --git a/src/Model/Event/TableFindIndexBefore.php b/src/Model/Event/TableFindIndexBefore.php index d81e2a858a..26274ea13c 100644 --- a/src/Model/Event/TableFindIndexBefore.php +++ b/src/Model/Event/TableFindIndexBefore.php @@ -92,9 +92,9 @@ private function setQuery(Query $query) * @param \Cake\ORM\Query $query Query * @param \App\Model\Table\Dto\FindIndexOptions $options Options * @param \Cake\ORM\Table $table Table - * @return self + * @return \App\Model\Event\TableFindIndexBefore */ - public static function create(Query $query, FindIndexOptions $options, Table $table): self + public static function create(Query $query, FindIndexOptions $options, Table $table) { return new static(static::EVENT_NAME, $table, [ 'query' => $query, diff --git a/src/Model/Table/AvatarsTable.php b/src/Model/Table/AvatarsTable.php index d9b94b9361..a693c8a095 100644 --- a/src/Model/Table/AvatarsTable.php +++ b/src/Model/Table/AvatarsTable.php @@ -193,32 +193,16 @@ public function afterDelete(Event $event, Avatar $avatar, \ArrayObject $options) * Used mainly to populate an avatar when no there is no result with the default avatar url. * * @param \Cake\Collection\CollectionInterface $avatars list of avatars (\App\Model\Entity\Avatar) - * @param bool $isHydrationEnabled if hydration is enabled, return an Avatar object, otherwise an array * @return mixed + * @deprecated the fallback avatar url is handled by the AvatarHelper. */ - private static function formatResults(CollectionInterface $avatars, bool $isHydrationEnabled) + public static function formatResults(CollectionInterface $avatars) { - return $avatars->map(function ($avatar) use ($isHydrationEnabled) { - $sizes = Configure::read('FileStorage.imageSizes.Avatar'); - $url = []; - // Add path for each available size. - foreach ($sizes as $size => $filters) { - $url[$size] = AvatarHelper::getAvatarUrl([ - 'id' => $avatar['id'] ?? null, - ], $size); - } - + return $avatars->map(function ($avatar) { if (empty($avatar)) { // If avatar is empty, we instantiate one. // The virtual field will take care of retrieving the default avatar. - $avatar = $isHydrationEnabled ? new Avatar() : []; - } - - if ($avatar instanceof Avatar) { - $avatar->setVirtual(['url'], true); - $avatar->set('url', (object)$url); - } else { - $avatar['url'] = $url; + $avatar = new Avatar(); } return $avatar; @@ -238,8 +222,8 @@ public static function addContainAvatar(): array // Formatter for empty avatars. return $q ->select(['Avatars.id', 'Avatars.profile_id', 'Avatars.created', 'Avatars.modified']) - ->formatResults(function (CollectionInterface $avatars, Query $mainQuery) { - return AvatarsTable::formatResults($avatars, $mainQuery->isHydrationEnabled()); + ->formatResults(function (CollectionInterface $avatars) { + return AvatarsTable::formatResults($avatars); }); }, ]; diff --git a/src/Model/Traits/Resources/ResourcesFindersTrait.php b/src/Model/Traits/Resources/ResourcesFindersTrait.php index f9c4e218a0..01d924b680 100644 --- a/src/Model/Traits/Resources/ResourcesFindersTrait.php +++ b/src/Model/Traits/Resources/ResourcesFindersTrait.php @@ -55,6 +55,7 @@ public function findIndex(string $userId, ?array $options = []) $event = TableFindIndexBefore::create($query, $findIndexOptions, $this); + /** @var \App\Model\Event\TableFindIndexBefore $event */ $this->getEventManager()->dispatch($event); // Filter out deleted resources diff --git a/tests/Lib/Model/EmailQueueTrait.php b/tests/Lib/Model/EmailQueueTrait.php index 945388d958..0b8102a438 100644 --- a/tests/Lib/Model/EmailQueueTrait.php +++ b/tests/Lib/Model/EmailQueueTrait.php @@ -153,7 +153,6 @@ protected function assertEmailInBatchContains( ): void { if ($htmlSpecialChar) { $string = h($string); - $string = htmlspecialchars($string); } $this->assertStringContainsString($string, $this->renderEmail($i), $message); } @@ -174,7 +173,6 @@ protected function assertEmailInBatchNotContains( ): void { if ($htmlSpecialChar) { $string = h($string); - $string = htmlspecialchars($string); } $this->assertStringNotContainsString($string, $this->renderEmail($i), $message); } diff --git a/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php b/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php index 4730dc5c2b..3e5fa414a0 100644 --- a/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php +++ b/tests/TestCase/Controller/Avatars/AvatarsViewControllerTest.php @@ -111,6 +111,13 @@ public function testViewAvatarsViewController_ViewExistentAvatar(string $format) $this->get('avatars/view/' . $avatar->id . '/' . $format . AvatarHelper::IMAGE_EXTENSION); $this->assertResponseEquals($expectedFileContent); + + // Ensure that the virtual field is correctly constructed. + $virtualField = [ + AvatarsConfigurationService::FORMAT_MEDIUM => AvatarHelper::getAvatarUrl($avatar->toArray(), AvatarsConfigurationService::FORMAT_MEDIUM), + AvatarsConfigurationService::FORMAT_SMALL => AvatarHelper::getAvatarUrl($avatar->toArray()), + ]; + $this->assertSame($virtualField, $avatar->url); } /** @@ -135,6 +142,13 @@ public function testAvatarsViewController_ViewExistentAvatarWithDeletedFile(stri $this->get('avatars/view/' . $avatar->id . '/' . $format . AvatarHelper::IMAGE_EXTENSION); $this->assertResponseEquals($expectedFileContent); + + // Ensure that the virtual field is correctly constructed. + $virtualField = [ + AvatarsConfigurationService::FORMAT_MEDIUM => AvatarHelper::getAvatarUrl($avatar->toArray(), AvatarsConfigurationService::FORMAT_MEDIUM), + AvatarsConfigurationService::FORMAT_SMALL => AvatarHelper::getAvatarUrl($avatar->toArray()), + ]; + $this->assertSame($virtualField, $avatar->url); } /** diff --git a/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php b/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php index 6fd6563c64..e76abb1b15 100644 --- a/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php +++ b/tests/TestCase/Model/Table/Avatars/AddContainAvatarTest.php @@ -35,18 +35,9 @@ public function setUp(): void { parent::setUp(); $this->Users = $this->fetchTable('Users'); - $this->loadRoutes(); } - public function enableHydration(): array - { - return [[true], [false]]; - } - - /** - * @dataProvider enableHydration - */ - public function testAvatarsTableAddContainAvatar_Should_Not_Retrieve_Avatar_Data(bool $isHydrationEnabled) + public function testAvatarsTableAddContainAvatar_Should_Not_Retrieve_Avatar_Data() { $user = UserFactory::make()->withAvatar()->user()->persist(); @@ -56,18 +47,14 @@ public function testAvatarsTableAddContainAvatar_Should_Not_Retrieve_Avatar_Data ->contain([ 'Profiles' => AvatarsTable::addContainAvatar(), ]) - ->enableHydration($isHydrationEnabled) ->firstOrFail(); - $this->assertSame($user->profile->avatar->id, $retrievedUser['profile']['avatar']['id']); + $this->assertSame($user->profile->avatar->id, $retrievedUser->profile->avatar->id); $this->assertNotNull($user->profile->avatar->data); - $this->assertNull($retrievedUser['profile']['avatar']['data'] ?? null); + $this->assertNull($retrievedUser->profile->avatar->data ?? null); } - /** - * @dataProvider enableHydration - */ - public function testAvatarsTableAddContainAvatar_On_Empty_Avatar(bool $isHydrationEnabled) + public function testAvatarsTableAddContainAvatar_On_Empty_Avatar() { $user = UserFactory::make()->user()->persist(); @@ -77,17 +64,12 @@ public function testAvatarsTableAddContainAvatar_On_Empty_Avatar(bool $isHydrati ->contain([ 'Profiles' => AvatarsTable::addContainAvatar(), ]) - ->enableHydration($isHydrationEnabled) ->firstOrFail(); $this->assertNull($user->profile->avatar->id ?? null); $this->assertNull($user->profile->avatar->data ?? null); - $this->assertNull($retrievedUser['profile']['avatar']['id'] ?? null); - $this->assertNull($retrievedUser['profile']['avatar']['data'] ?? null); - if ($isHydrationEnabled) { - $this->assertIsObject($retrievedUser['profile']['avatar']['url']); - } else { - $this->assertIsArray($retrievedUser['profile']['avatar']['url']); - } + $this->assertNull($retrievedUser->profile->avatar->id ?? null); + $this->assertNull($retrievedUser->profile->avatar->data ?? null); + $this->assertIsArray($retrievedUser->profile->avatar->url); } } diff --git a/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php b/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php index 12235959a7..f77a3ce571 100644 --- a/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php +++ b/tests/TestCase/Model/Table/Permissions/FindViewAcoPermissionsTest.php @@ -19,7 +19,6 @@ use App\Model\Table\PermissionsTable; use App\Test\Lib\AppTestCase; -use App\Test\Lib\Model\AvatarsModelTrait; use App\Utility\UuidFactory; use Cake\ORM\TableRegistry; use Cake\Utility\Hash; @@ -27,8 +26,6 @@ class FindViewAcoPermissionsTest extends AppTestCase { - use AvatarsModelTrait; - public $fixtures = ['app.Base/Groups', 'app.Base/Permissions', 'app.Base/Profiles', 'app.Base/Resources', 'app.Base/Users']; /** @@ -84,7 +81,12 @@ public function testContainUserProfileAvatar() $permission = Hash::extract($permissions->toArray(), '{n}[aro=User]')[0]; $this->assertPermissionAttributes($permission); $this->assertProfileAttributes($permission->user->profile); - $this->assertAvatarAttributes($permission->user->profile->avatar); + $this->assertObjectHasAttribute('avatar', $permission->user->profile); + $this->assertArrayHasKey('url', $permission->user->profile->avatar); + $this->assertArrayHasKey('medium', $permission->user->profile->avatar->url); + $this->assertArrayHasKey('small', $permission->user->profile->avatar->url); + $this->assertStringContainsString('/img/avatar/user_medium.png', $permission->user->profile->avatar->url['medium']); + $this->assertStringContainsString('/img/avatar/user.png', $permission->user->profile->avatar->url['small']); } public function testPermissions() From dfa5681eb62dc20cc78ed80f5ef301c944419ca8 Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Mon, 20 Nov 2023 13:26:04 +0100 Subject: [PATCH 15/16] PB-28730 v4.4.1-test.3 --- CHANGELOG.md | 4 ++++ config/version.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c61499372e..66525695f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [4.4.1-test.3] - 2023-11-20 +### Fixed +- PB-27616 Revert performance improvements as further investigation is required + ## [4.4.1-test.2] - 2023-11-20 ### Maintenance - PB-28521 Alter the gpgkeys.uid column length to 769 diff --git a/config/version.php b/config/version.php index 1826c0ca0d..6880ec1d39 100644 --- a/config/version.php +++ b/config/version.php @@ -1,7 +1,7 @@ [ - 'version' => '4.4.1-test.2', + 'version' => '4.4.1-test.3', 'name' => 'TBD', ], 'php' => [ From a998e9ea0b83be7caf9b58af2791cd9f2a72fc93 Mon Sep 17 00:00:00 2001 From: Juan Pablo Ramirez Date: Mon, 20 Nov 2023 15:03:23 +0100 Subject: [PATCH 16/16] v4.4.1 --- CHANGELOG.md | 11 +++++++++++ RELEASE_NOTES.md | 37 +++++++++---------------------------- config/version.php | 4 ++-- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66525695f1..30e36ff803 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [4.4.1] - 2023-11-20 +### Improved +- PB-28521 Alter the gpgkeys.uid column length to 769 to enable the synchronisation of user with very long names + +### Fixed +- PB-27957 As an administrator I should be notified that an administrator was deleted only when an administrator has been deleted, and not a regular user + +### Maintenance +- PB-27927 Remove unused user_agents table +- PB-28616 Refactor the email digest plugin code for easier usage and maintainability + ## [4.4.1-test.3] - 2023-11-20 ### Fixed - PB-27616 Revert performance improvements as further investigation is required diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c1a6fb25fb..4d9987c429 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,37 +1,18 @@ -Release song: https://www.youtube.com/watch?v=6Ejga4kJUts +Release song: https://youtu.be/RbmS3tQJ7Os?si=lp8QM5B-R65C8Jek -Version 4.4 of the Community Edition has launched with new capabilities and improvements. +Passbolt v4.4.1 is a maintenance release aimed at addressing issues reported by the community, which were introduced in the previous release. -With this release, users are able to manage TOTPs directly from the browser, providing an extended TOTP experience across all their devices. They can now be created, deleted, organised and shared with others just like any other resource type. +The update addresses an issue concerning user roles in email notifications. Previously, administrators received notifications when another administrator was deleted. However, the deletion of any user, regardless of their administrator status, was incorrectly reported as an administrator deletion. This issue has been resolved. -Another highlight of this release, administrators now have the ability to suspend/unsuspend users. This new feature will offer administrators with more control over access management of their instance. By example, they will be able to prevent access to the passbolt instance for users in temporary leave, therefore enforce company policies. - -And that's not all – a number of fixes and enhancements have been implemented to improve user experience. Among them, notification emails are now aggregated in certain cases, including limiting emails when a user imports a large amount of passwords. - -If you’re a system operator, please note that using older PHP versions will now trigger a healthcheck warning. Support for PHP 7.4 and 8.0 will be discontinued soon. Admins are encouraged to upgrade to PHP 8.1 or higher and use the latest version of the passbolt API. - -Get the most out of passbolt – upgrade to version 4.4. Thanks for continuing to support passbolt and for being part of the community! - - -## [4.4.0] - 2023-11-07 -### Added -- PB-27773 As an administrator I can deny access to the mobile setup screen with RBAC -- PB-27951 As system operator I should be warned in the healthcheck when using PHP < 8.1, as support for PHP versions 7.4 and 8.0 will soon be removed +We extend our gratitude to the community members who reported these issues. Your support and patience are greatly appreciated. +## [4.4.1] - 2023-11-20 ### Improved -- PB-27948 Guest identification by their username should be case-insensitive, unless specified in configuration -- PB-27957 Send notifications to all administrators when an administrator is deleted -- PB-27941 Send notifications to administrators when an administrator loses its administrator role -- PB-28171 Enable the email digest by default - -### Security -- PB-28274 Fixes an XSS Security issue with mail content sanitization +- PB-28521 Alter the gpgkeys.uid column length to 769 to enable the synchronisation of user with very long names ### Fixed -- PB-25477 As an administrator, I should be able to recreate a user with an email that exists in the db via the command line -- PB-27799 As an administrator installing passbolt on PostgreSQL, the database encoding should be set to utf-8 -- PB-27857 Fix help site release notes automation by adding flavour on help site release notes merge request +- PB-27957 As an administrator I should be notified that an administrator was deleted only when an administrator has been deleted, and not a regular user ### Maintenance -- PB-27932 Improve code static by using cakedccakephp/phpstan -- PB-28079 Remove deprecation warnings from the test suite +- PB-27927 Remove unused user_agents table +- PB-28616 Refactor the email digest plugin code for easier usage and maintainability diff --git a/config/version.php b/config/version.php index 6880ec1d39..7ec1ae1658 100644 --- a/config/version.php +++ b/config/version.php @@ -1,8 +1,8 @@ [ - 'version' => '4.4.1-test.3', - 'name' => 'TBD', + 'version' => '4.4.1', + 'name' => 'Gimme Shelter', ], 'php' => [ 'minVersion' => '7.4',