diff --git a/lib/Db/AliasMapper.php b/lib/Db/AliasMapper.php index 2abae420ac..6590cf700a 100644 --- a/lib/Db/AliasMapper.php +++ b/lib/Db/AliasMapper.php @@ -12,6 +12,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use function array_map; @@ -106,12 +107,7 @@ public function deleteAll($accountId): void { /** * Delete all provisioned aliases for the given uid * - * Exception for Nextcloud 20: \Doctrine\DBAL\DBALException - * Exception for Nextcloud 21 and newer: \OCP\DB\Exception - * - * @TODO: Change throws to \OCP\DB\Exception once Mail does not support Nextcloud 20. - * - * @throws \Exception + * @throws Exception */ public function deleteProvisionedAliasesByUid(string $uid): void { $qb = $this->db->getQueryBuilder(); diff --git a/lib/Db/MailAccountMapper.php b/lib/Db/MailAccountMapper.php index c17098dca5..655f08d95f 100644 --- a/lib/Db/MailAccountMapper.php +++ b/lib/Db/MailAccountMapper.php @@ -160,6 +160,9 @@ public function save(MailAccount $account): MailAccount { return $this->update($account); } + /** + * @throws Exception + */ public function deleteProvisionedAccounts(int $provisioningId): void { $qb = $this->db->getQueryBuilder(); @@ -169,6 +172,9 @@ public function deleteProvisionedAccounts(int $provisioningId): void { $delete->executeStatement(); } + /** + * @throws Exception + */ public function deleteProvisionedAccountsByUid(string $uid): void { $qb = $this->db->getQueryBuilder(); diff --git a/lib/Service/Provisioning/Manager.php b/lib/Service/Provisioning/Manager.php index ce117cc423..3d88438e67 100644 --- a/lib/Service/Provisioning/Manager.php +++ b/lib/Service/Provisioning/Manager.php @@ -10,6 +10,7 @@ namespace OCA\Mail\Service\Provisioning; use Horde_Mail_Rfc822_Address; +use OCA\Mail\AppInfo\Application; use OCA\Mail\Db\Alias; use OCA\Mail\Db\AliasMapper; use OCA\Mail\Db\MailAccount; @@ -19,8 +20,10 @@ use OCA\Mail\Db\TagMapper; use OCA\Mail\Exception\ValidationException; use OCA\Mail\Service\AccountService; +use OCP\App\IAppManager; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\DB\Exception; use OCP\ICacheFactory; use OCP\IUser; use OCP\IUserManager; @@ -30,6 +33,8 @@ class Manager { public const MAIL_PROVISIONINGS = 'mail_provisionings'; + /** @var IAppManager */ + private $appManager; /** @var IUserManager */ private $userManager; @@ -58,6 +63,7 @@ class Manager { private $cacheFactory; public function __construct( + IAppManager $appManager, IUserManager $userManager, ProvisioningMapper $provisioningMapper, MailAccountMapper $mailAccountMapper, @@ -69,6 +75,7 @@ public function __construct( ICacheFactory $cacheFactory, private AccountService $accountService, ) { + $this->appManager = $appManager; $this->userManager = $userManager; $this->provisioningMapper = $provisioningMapper; $this->mailAccountMapper = $mailAccountMapper; @@ -125,7 +132,7 @@ public function provision(): int { * A alias is orphaned if not listed in newAliases anymore * (=> the provisioning configuration does contain it anymore) * - * @throws \OCP\DB\Exception + * @throws Exception */ private function deleteOrphanedAliases(string $userId, int $accountId, array $newAliases): void { $existingAliases = $this->aliasMapper->findAll($accountId, $userId); @@ -139,7 +146,7 @@ private function deleteOrphanedAliases(string $userId, int $accountId, array $ne /** * Create new aliases for the given account. * - * @throws \OCP\DB\Exception + * @throws Exception */ private function createNewAliases(string $userId, int $accountId, array $newAliases, string $displayName, string $accountEmail): void { foreach ($newAliases as $newAlias) { @@ -186,6 +193,26 @@ public function ldapAliasesIntegration(Provisioning $provisioning, IUser $user): * @param Provisioning[] $provisionings */ public function provisionSingleUser(array $provisionings, IUser $user): bool { + $unprovisionUser = function (string $userUid) : void { + try { + /** @noinspection PhpUnhandledExceptionInspection */ + $this->aliasMapper->deleteProvisionedAliasesByUid($userUid); + /** @noinspection PhpUnhandledExceptionInspection */ + $this->mailAccountMapper->deleteProvisionedAccountsByUid($userUid); + } catch (Exception $e) { + $this->logger->warning( + "Error during deletion of mail provisioning profile for user with UID {$userUid}", + ['exception' => $e] + ); + } + }; + + if (!$this->appManager->isEnabledForUser(Application::APP_ID, $user)) { + $unprovisionUser($user->getUID()); + + return false; + } + $provisioning = $this->findMatchingConfig($provisionings, $user); if ($provisioning === null) { @@ -203,8 +230,7 @@ public function provisionSingleUser(array $provisionings, IUser $user): bool { if ($e instanceof MultipleObjectsReturnedException) { // This is unlikely to happen but not impossible. // Let's wipe any existing accounts and start fresh - $this->aliasMapper->deleteProvisionedAliasesByUid($user->getUID()); - $this->mailAccountMapper->deleteProvisionedAccountsByUid($user->getUID()); + $unprovisionUser($user->getUID()); } // Fine, then we create a new one @@ -244,7 +270,7 @@ public function provisionSingleUser(array $provisionings, IUser $user): bool { /** * @throws ValidationException - * @throws \OCP\DB\Exception + * @throws Exception */ public function newProvisioning(array $data): Provisioning { $provisioning = $this->provisioningMapper->validate($data); @@ -258,7 +284,7 @@ public function newProvisioning(array $data): Provisioning { /** * @throws ValidationException - * @throws \OCP\DB\Exception + * @throws Exception */ public function updateProvisioning(array $data): void { $provisioning = $this->provisioningMapper->validate($data); diff --git a/tests/Unit/Service/Provisioning/ManagerTest.php b/tests/Unit/Service/Provisioning/ManagerTest.php index 3b29412b45..6bfd7105c8 100644 --- a/tests/Unit/Service/Provisioning/ManagerTest.php +++ b/tests/Unit/Service/Provisioning/ManagerTest.php @@ -61,6 +61,30 @@ public function testProvisionSkipWithoutConfigurations(): void { $this->assertEquals(0, $count); } + public function testDisabledAppDoesUnprovision() { + /** @var IUser|MockObject $user */ + $user = $this->createConfiguredMock(IUser::class, [ + 'getEmailAddress' => 'bruce.wayne@batman.com', + 'getUID' => 'bruce' + ]); + $configs = [new Provisioning()]; + $this->mock->getParameter('appManager') + ->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(false); + $this->mock->getParameter('aliasMapper') + ->expects($this->once()) + ->method('deleteProvisionedAliasesByUid') + ->with($user->getUID()); + $this->mock->getParameter('mailAccountMapper') + ->expects($this->once()) + ->method('deleteProvisionedAccountsByUid') + ->with($user->getUID()); + + $result = $this->manager->provisionSingleUser($configs, $user); + $this->assertFalse($result); + } + public function testUpdateProvisionSingleUser() { /** @var IUser|MockObject $user */ $user = $this->createConfiguredMock(IUser::class, [ @@ -74,6 +98,10 @@ public function testUpdateProvisionSingleUser() { $configs = [$config]; $mailAccount = new MailAccount(); $mailAccount->setId(1000); + $this->mock->getParameter('appManager') + ->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(true); $this->mock->getParameter('mailAccountMapper') ->expects($this->once()) ->method('findProvisionedAccount') @@ -100,6 +128,10 @@ public function testProvisionSingleUser() { $config->setProvisioningDomain('batman.com'); $config->setEmailTemplate('%USER%@batman.com'); $configs = [$config]; + $this->mock->getParameter('appManager') + ->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(true); $this->mock->getParameter('mailAccountMapper') ->expects($this->once()) ->method('findProvisionedAccount') @@ -133,6 +165,10 @@ public function testUpdateProvisionSingleUserWithWildcard() { $configs = [$config]; $mailAccount = new MailAccount(); $mailAccount->setId(1000); + $this->mock->getParameter('appManager') + ->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(true); $this->mock->getParameter('mailAccountMapper') ->expects($this->once()) ->method('findProvisionedAccount') @@ -159,6 +195,10 @@ public function testProvisionSingleUserWithWildcard() { $config->setProvisioningDomain('*'); $config->setEmailTemplate('%USER%@batman.com'); $configs = [$config]; + $this->mock->getParameter('appManager') + ->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(true); $this->mock->getParameter('mailAccountMapper') ->expects($this->once()) ->method('findProvisionedAccount') @@ -186,6 +226,10 @@ public function testProvisionSingleUserNoDomainMatch() { $config->setProvisioningDomain('arkham-asylum.com'); $config->setEmailTemplate('%USER%@batman.com'); $configs = [$config]; + $this->mock->getParameter('appManager') + ->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(true); $this->mock->getParameter('mailAccountMapper') ->expects($this->never()) ->method('findProvisionedAccount');