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

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

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

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' => '[email protected]',
+			'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');