From b7d464f9eb4a5dd3223fcce9d0baf0992b459843 Mon Sep 17 00:00:00 2001 From: Guillaume DUPRAZ-CANARD Date: Tue, 25 Aug 2026 15:49:18 +0200 Subject: [PATCH] fix: apply original user rules on alias creation --- app/src/Controller/UserController.php | 3 +- app/src/Entity/User.php | 2 +- app/src/Service/UserService.php | 111 +++++++++++++++++--- app/tests/Controller/UserControllerTest.php | 72 +++++++++++++ app/tests/MessageHelper.php | 91 ++++++++++++++++ 5 files changed, 261 insertions(+), 18 deletions(-) create mode 100644 app/tests/MessageHelper.php diff --git a/app/src/Controller/UserController.php b/app/src/Controller/UserController.php index 94cecf7f..d3781cf2 100644 --- a/app/src/Controller/UserController.php +++ b/app/src/Controller/UserController.php @@ -458,8 +458,9 @@ public function newUserAlias(Request $request, UserService $userService, GroupSe $this->em->persist($user); $this->em->flush(); - $userService->updateAliasGroupsAndPolicyFromUser($user->getOriginalUser()); + $userService->updateAliasGroupsAndPolicyFromUser($user); $groupService->updateSenderRules(); + $return = [ 'status' => 'success', 'message' => $this->translator->trans('Generics.flash.addSuccess'), diff --git a/app/src/Entity/User.php b/app/src/Entity/User.php index 9f8bfd69..8c007183 100644 --- a/app/src/Entity/User.php +++ b/app/src/Entity/User.php @@ -86,7 +86,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface #[ORM\ManyToOne(targetEntity: User::class, inversedBy: 'aliases')] #[ORM\JoinColumn(name: 'original_user_id', nullable: true, onDelete: 'CASCADE')] - private ?User $originalUser; + private ?User $originalUser = null; /** * @var Collection diff --git a/app/src/Service/UserService.php b/app/src/Service/UserService.php index ef3bec3a..e2a94ca9 100644 --- a/app/src/Service/UserService.php +++ b/app/src/Service/UserService.php @@ -3,6 +3,8 @@ namespace App\Service; use App\Entity\Domain; +use App\Entity\Group; +use App\Entity\SenderRule; use App\Entity\User; use App\Repository\GroupRepository; use App\Repository\UserRepository; @@ -58,33 +60,110 @@ public function updateUsersPolicyfromDomain(Domain $domain): void } /** - * Update groups and policy for user aliases + * Update groups and policy for user and its aliases */ - public function updateAliasGroupsAndPolicyFromUser(User $originalUser): void + public function updateAliasGroupsAndPolicyFromUser(User $user): void { - $originalUserGroups = $originalUser->getGroups()->toArray(); + $parentUser = $user->getOriginalUser(); + if ($parentUser) { + // If there is a parent, current user is an alias => update using groups and rules from parent + $this->updateGroups($user, $parentUser->getGroups()->toArray()); + $this->updateSenderRules($user, $parentUser->getSenderRules()->toArray()); + } else { + // Otherwise it's an original user + $originalUserGroups = $user->getGroups()->toArray(); + $originalUserSenderRules = $user->getSenderRules()->toArray(); - $aliases = $originalUser->getAliases(); + $aliases = $user->getAliases(); + foreach ($aliases as $alias) { + $alias->setPolicy($user->getPolicy()); - foreach ($aliases as $alias) { - $alias->setPolicy($originalUser->getPolicy()); + $this->updateGroups($alias, $originalUserGroups); + $this->updateSenderRules($alias, $originalUserSenderRules); - $aliasGroups = $alias->getGroups()->toArray(); + $this->em->persist($alias); + } + } - $groupsToAdd = array_diff($originalUserGroups, $aliasGroups); - $groupsToRemove = array_diff($aliasGroups, $originalUserGroups); + $this->em->flush(); + } - foreach ($groupsToAdd as $group) { - $alias->addGroup($group); - } + /** + * @param array $userGroups + */ + private function updateGroups(User $user, array $userGroups): void + { + $aliasGroups = $user->getGroups()->toArray(); - foreach ($groupsToRemove as $group) { - $alias->removeGroup($group); + $groupsToAdd = array_udiff( + $userGroups, + $aliasGroups, + fn(Group $a, Group $b) => $a->getId() <=> $b->getId() + ); + + $groupsToRemove = array_udiff( + $aliasGroups, + $userGroups, + fn(Group $a, Group $b) => $a->getId() <=> $b->getId() + ); + + foreach ($groupsToAdd as $group) { + $user->addGroup($group); + } + + foreach ($groupsToRemove as $group) { + $user->removeGroup($group); + } + } + + /** + * @param array $userSenderRules + */ + private function updateSenderRules(User $user, array $userSenderRules): void + { + $existingRules = $user->getSenderRules()->toArray(); + $existingMap = []; + foreach ($existingRules as $rule) { + $existingMap[$this->senderRuleKey($rule)] = $rule; + } + + $sourceMap = []; + foreach ($userSenderRules as $rule) { + $sourceMap[$this->senderRuleKey($rule)] = $rule; + } + + // Remove rules that shouldn't exist on this user. + foreach ($existingRules as $existingRule) { + $key = $this->senderRuleKey($existingRule); + + if (!isset($sourceMap[$key])) { + $user->removeSenderRule($existingRule); + $this->em->remove($existingRule); } + } - $this->em->persist($alias); + // Add copies of missing rules. + foreach ($sourceMap as $key => $sourceRule) { + $existingRule = $existingMap[$this->senderRuleKey($sourceRule)] ?? null; + + if ($existingRule === null) { + // As a SenderRule is associated to a unique User, we have to clone it and associate to new User. + $newRule = new SenderRule($user, $sourceRule->getSenderRuleAddress()); + $newRule->setPriority($sourceRule->getPriority()); + $newRule->setWb($sourceRule->getWb()); + + $this->em->persist($newRule); + $user->addSenderRule($newRule); + } } + } - $this->em->flush(); + private function senderRuleKey(SenderRule $rule): string + { + return sprintf( + '%d:%d', + $rule->getSenderRuleAddress()->getId(), + $rule->getPriority() + ); } } diff --git a/app/tests/Controller/UserControllerTest.php b/app/tests/Controller/UserControllerTest.php index 3716075e..9d503055 100644 --- a/app/tests/Controller/UserControllerTest.php +++ b/app/tests/Controller/UserControllerTest.php @@ -2,8 +2,12 @@ namespace App\Tests\Controller; +use App\Entity\SenderRule; use App\Tests\Factory\DomainFactory; +use App\Tests\Factory\RuleAddressFactory; +use App\Tests\Factory\SenderRuleFactory; use App\Tests\Factory\UserFactory; +use App\Tests\MessageHelper; use App\Tests\SessionHelper; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\HttpFoundation\Request; @@ -15,6 +19,7 @@ class UserControllerTest extends WebTestCase { use Factories; + use MessageHelper; use ResetDatabase; use SessionHelper; @@ -196,4 +201,71 @@ public function testAdminCannotCreateUsersWithIncorrectCsrfToken(): void // But no user created (1 because of `@domain.tld`, and one for the admin) self::assertSame(2, UserFactory::count()); } + + public function testCreateAnAliasForAUserWithSenderRulesShouldApplyThoseRules(): void + { + $client = static::createClient(); + $domain = DomainFactory::createOne(); + $superAdmin = UserFactory::new()->superAdmin()->create(); + $client->loginUser($superAdmin); + $sender = UserFactory::new()->user($domain)->create(); + $recipient = UserFactory::new()->user($domain)->create(); + [$senderAddress, $userAddress] = $this->setupAddresses($sender, $recipient); + $senderRuleAddress = RuleAddressFactory::new()->create([ + 'email' => $senderAddress->getEmail(), + 'priority' => 7, + ]); + $senderRule = SenderRuleFactory::new()->create([ + 'user' => $sender, + 'senderRuleAddress' => $senderRuleAddress, + 'wb' => 'accept', + 'type' => SenderRule::TYPE_USER, + 'priority' => SenderRule::PRIORITY_USER, + ]); + self::assertCount(1, $sender->getSenderRules()); + $initialCount = UserFactory::count(); + // The rule is still associated to the user + $em = self::getContainer()->get('doctrine')->getManager(); + $em->refresh($recipient); + + $fullname = 'test fullname'; + $alias = 'other@' . $domain->getDomain(); + $username = $alias; + $client->request(Request::METHOD_POST, '/admin/users/newAlias', [ + 'user' => [ + '_token' => $this->generateCsrfToken($client, 'user'), + 'fullname' => $fullname, + 'username' => $username, + 'email' => $alias, + 'originalUser' => $sender->getId(), + 'report' => 1, + ], + ]); + + $content = $client->getResponse()->getContent(); + self::assertNotFalse($content); + self::assertResponseIsSuccessful(); + self::assertJsonStringEqualsJsonString( + '{"status":"success","message":"Added successfully!"}', + $content, + ); + // Alias has been created + self::assertSame($initialCount + 1, UserFactory::count()); + $newAlias = UserFactory::last(); + self::assertSame($fullname, $newAlias->getFullname()); + self::assertSame($alias, $newAlias->getUsername()); + self::assertSame($alias, $newAlias->getEmail()); + // The rule has been associated to the new alias + self::assertSame(1, $newAlias->getSenderRules()->count()); + $aliasSenderRule = $newAlias->getSenderRules()->first(); + self::assertNotFalse($aliasSenderRule); + self::assertSame($newAlias->getId(), $aliasSenderRule->getUser()->getId()); + self::assertSame($senderRule->getSenderRuleAddress(), $aliasSenderRule->getSenderRuleAddress()); + self::assertSame($senderRule->getPriority(), $aliasSenderRule->getPriority()); + // The rule is still associated to the user + $em = self::getContainer()->get('doctrine')->getManager(); + $em->refresh($sender); + self::assertSame(1, $sender->getSenderRules()->count()); + self::assertSame($senderRule, $sender->getSenderRules()->first()); + } } diff --git a/app/tests/MessageHelper.php b/app/tests/MessageHelper.php new file mode 100644 index 00000000..dedcd95a --- /dev/null +++ b/app/tests/MessageHelper.php @@ -0,0 +1,91 @@ + + */ + private function setupAddresses(User $sender, User $recipient): array + { + $addrR = AddressFactory::createOne([ + 'domain' => Url::reverseDomainName($recipient->getDomain()->getDomain()), + 'partitionTag' => 0, + 'email' => $recipient->getEmail(), + ]); + $addrS = AddressFactory::createOne([ + 'domain' => Url::reverseDomainName($sender->getDomain()->getDomain()), + 'partitionTag' => 0, + 'email' => $sender->getEmail(), + ]); + + return [ + $addrS, + $addrR, + ]; + } + + private function setupMail( + Address $sender, + Address $recipient, + ?string $subject = 'test', + ?int $status = null, + // TODO: add optional body to generate mail content + ): string { + $mailId = bin2hex(random_bytes(8)); + + $message = MessageFactory::createOne([ + 'partitionTag' => 0, + 'mailId' => $mailId, + 'senderAddress' => $sender, + 'subject' => $subject, + 'fromAddr' => $sender->getEmail(), + 'status' => $status, + ]); + + MessageRecipientFactory::new()->create([ + 'message' => $message, + 'partitionTag' => 0, + 'mailId' => $mailId, + 'status' => $status, + 'address' => $recipient, + 'rseqnum' => 1, + 'isLocal' => 'N', + 'content' => 'S', + 'ds' => 'D', + 'bl' => 'N', + 'wl' => 'N', + 'bspamLevel' => -1.2, + 'smtpResp' => '250 2.7.0 Ok, discarded, id=00045-01 - spam', + 'sendCaptcha' => 0, + 'amavisOutput' => null, + 'amavisReleaseStartedAt' => null, + 'amavisReleaseEndedAt' => null, + ]); + + $mailText = QuarantineFactory::generateMailText($mailId, [ + 'subject' => $subject, + 'from' => $sender->getEmail(), + 'to' => [$recipient->getEmail()], + ]); + + QuarantineFactory::new()->create([ + 'partitionTag' => 0, + 'mailId' => $mailId, + 'message' => $message, + 'chunkInd' => 0, + 'mailText' => $mailText, + ]); + + return $mailId; + } +}