fix(provisioning): Do not provision mail account for users without access to mail app

lib/Db/AliasMapper.php

@@ -106,12 +106,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 \OCP\DB\Exception
 	 */
 	public function deleteProvisionedAliasesByUid(string $uid): void {
 		$qb = $this->db->getQueryBuilder();

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();
 

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;
@@ -20,9 +21,10 @@
 use OCA\Mail\Exception\ValidationException;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\Classification\ClassificationSettingsService;
+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;
@@ -32,6 +34,9 @@
 
 class Manager {
 	public const MAIL_PROVISIONINGS = 'mail_provisionings';
+	/** @var IAppManager */
+	private $appManager;
+
 	/** @var IUserManager */
 	private $userManager;
 
@@ -60,6 +65,7 @@ class Manager {
 	private $cacheFactory;
 
 	public function __construct(
+		IAppManager $appManager,
 		IUserManager $userManager,
 		ProvisioningMapper $provisioningMapper,
 		MailAccountMapper $mailAccountMapper,
@@ -72,6 +78,7 @@ public function __construct(
 		private AccountService $accountService,
 		private ClassificationSettingsService $classificationSettingsService,
 	) {
+		$this->appManager = $appManager;
 		$this->userManager = $userManager;
 		$this->provisioningMapper = $provisioningMapper;
 		$this->mailAccountMapper = $mailAccountMapper;
@@ -185,10 +192,35 @@ public function ldapAliasesIntegration(Provisioning $provisioning, IUser $user):
 		return $provisioning;
 	}
 
+	/**
+	 * Delete all provisioned aliases and accounts for
+	 * a specific user UID.
+	 *
+	 * @param string $userUid
+	 * @return void
+	 */
+	public function unprovisionSingleUser(string $userUid) : void {
+		try {
+			$this->aliasMapper->deleteProvisionedAliasesByUid($userUid);
+			$this->mailAccountMapper->deleteProvisionedAccountsByUid($userUid);
+		} catch (Exception $e) {
+			$this->logger->warning(
+				"Error during deletion of mail provisioning profile for user with UID {$userUid}",
+				['exception' => $e]
+			);
+		}
+	}
+
 	/**
 	 * @param Provisioning[] $provisionings
 	 */
 	public function provisionSingleUser(array $provisionings, IUser $user): bool {
+		if (!$this->appManager->isEnabledForUser(Application::APP_ID, $user)) {
+			$this->unprovisionSingleUser($user->getUID());
+
+			return false;
+		}
+
 		$provisioning = $this->findMatchingConfig($provisionings, $user);
 
 		if ($provisioning === null) {
@@ -206,8 +238,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());
+				$this->unprovisionSingleUser($user->getUID());
 			}
 
 			// Fine, then we create a new one

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');