fix(apps-store): Remove apps from force-enabled state when uninstalled

If an app is force-enabled and then uninstalled the force-enabled state was kept. This is now removed, so when the app should be re-installed the compatibility should be reevaluated. Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de> Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
nextcloud · Nov 18, 2024 · 9e327a5 · 9e327a5
1 parent 4d138a1
commit 9e327a5
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 37 deletions.
diff --git a/apps/settings/lib/Controller/AppSettingsController.php b/apps/settings/lib/Controller/AppSettingsController.php
@@ -6,6 +6,7 @@
  */
 namespace OCA\Settings\Controller;
 
+use OC\App\AppManager;
 use OC\App\AppStore\Bundles\BundleFetcher;
 use OC\App\AppStore\Fetcher\AppDiscoverFetcher;
 use OC\App\AppStore\Fetcher\AppFetcher;
@@ -15,7 +16,6 @@
 use OC\App\Platform;
 use OC\Installer;
 use OCP\App\AppPathNotFoundException;
-use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
@@ -62,7 +62,7 @@ public function __construct(
 		private IL10N $l10n,
 		private IConfig $config,
 		private INavigationManager $navigationManager,
-		private IAppManager $appManager,
+		private AppManager $appManager,
 		private CategoryFetcher $categoryFetcher,
 		private AppFetcher $appFetcher,
 		private IFactory $l10nFactory,
@@ -592,6 +592,8 @@ public function uninstallApp(string $appId): JSONResponse {
 		$appId = $this->appManager->cleanAppId($appId);
 		$result = $this->installer->removeApp($appId);
 		if ($result !== false) {
+			// If this app was force enabled, remove the force-enabled-state
+			$this->appManager->removeOverwriteNextcloudRequirement($appId);
 			$this->appManager->clearAppsCache();
 			return new JSONResponse(['data' => ['appid' => $appId]]);
 		}
@@ -631,7 +633,7 @@ private function sortApps($a, $b) {
 
 	public function force(string $appId): JSONResponse {
 		$appId = $this->appManager->cleanAppId($appId);
-		$this->appManager->ignoreNextcloudRequirementForApp($appId);
+		$this->appManager->overwriteNextcloudRequirement($appId);
 		return new JSONResponse();
 	}
 }
diff --git a/apps/settings/tests/Controller/AppSettingsControllerTest.php b/apps/settings/tests/Controller/AppSettingsControllerTest.php
@@ -6,13 +6,13 @@
  */
 namespace OCA\Settings\Tests\Controller;
 
+use OC\App\AppManager;
 use OC\App\AppStore\Bundles\BundleFetcher;
 use OC\App\AppStore\Fetcher\AppDiscoverFetcher;
 use OC\App\AppStore\Fetcher\AppFetcher;
 use OC\App\AppStore\Fetcher\CategoryFetcher;
 use OC\Installer;
 use OCA\Settings\Controller\AppSettingsController;
-use OCP\App\IAppManager;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -47,8 +47,7 @@ class AppSettingsControllerTest extends TestCase {
 	private $config;
 	/** @var INavigationManager|MockObject */
 	private $navigationManager;
-	/** @var IAppManager|MockObject */
-	private $appManager;
+	private AppManager&MockObject $appManager;
 	/** @var CategoryFetcher|MockObject */
 	private $categoryFetcher;
 	/** @var AppFetcher|MockObject */
@@ -83,7 +82,7 @@ protected function setUp(): void {
 			->willReturnArgument(0);
 		$this->config = $this->createMock(IConfig::class);
 		$this->navigationManager = $this->createMock(INavigationManager::class);
-		$this->appManager = $this->createMock(IAppManager::class);
+		$this->appManager = $this->createMock(AppManager::class);
 		$this->categoryFetcher = $this->createMock(CategoryFetcher::class);
 		$this->appFetcher = $this->createMock(AppFetcher::class);
 		$this->l10nFactory = $this->createMock(IFactory::class);

diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
@@ -1025,11 +1025,6 @@
       <code><![CDATA[getSettingsManager]]></code>
     </UndefinedInterfaceMethod>
   </file>
-  <file src="apps/settings/lib/Controller/AppSettingsController.php">
-    <UndefinedInterfaceMethod>
-      <code><![CDATA[ignoreNextcloudRequirementForApp]]></code>
-    </UndefinedInterfaceMethod>
-  </file>
   <file src="apps/settings/lib/Hooks.php">
     <InvalidArrayOffset>
       <code><![CDATA[[$user->getEMailAddress() => $user->getDisplayName()]]]></code>
@@ -1337,12 +1332,6 @@
       <code><![CDATA[!is_array($userIds)]]></code>
     </TypeDoesNotContainType>
   </file>
-  <file src="lib/private/App/AppManager.php">
-    <LessSpecificImplementedReturnType>
-      <code><![CDATA[array]]></code>
-      <code><![CDATA[array]]></code>
-    </LessSpecificImplementedReturnType>
-  </file>
   <file src="lib/private/App/AppStore/Fetcher/Fetcher.php">
     <TooManyArguments>
       <code><![CDATA[fetch]]></code>

diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
@@ -200,10 +200,6 @@ public function getEnabledAppsForUser(IUser $user) {
 		return array_keys($appsForUser);
 	}
 
-	/**
-	 * @param IGroup $group
-	 * @return array
-	 */
 	public function getEnabledAppsForGroup(IGroup $group): array {
 		$apps = $this->getInstalledAppsValues();
 		$appsForGroups = array_filter($apps, function ($enabled) use ($group) {
@@ -304,10 +300,6 @@ public function getAutoDisabledApps(): array {
 		return $this->autoDisabledApps;
 	}
 
-	/**
-	 * @param string $appId
-	 * @return array
-	 */
 	public function getAppRestriction(string $appId): array {
 		$values = $this->getInstalledAppsValues();
 
@@ -321,7 +313,6 @@ public function getAppRestriction(string $appId): array {
 		return json_decode($values[$appId], true);
 	}
 
-
 	/**
 	 * Check if an app is enabled for user
 	 *
@@ -410,12 +401,25 @@ public function isInstalled($appId) {
 		return isset($installedApps[$appId]);
 	}
 
-	public function ignoreNextcloudRequirementForApp(string $appId): void {
+	/**
+	 * Overwrite the `max-version` requirement for this app.
+	 */
+	public function overwriteNextcloudRequirement(string $appId): void {
 		$ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []);
 		if (!in_array($appId, $ignoreMaxApps, true)) {
 			$ignoreMaxApps[] = $appId;
-			$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
 		}
+		$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
+	}
+
+	/**
+	 * Remove the `max-version` overwrite for this app.
+	 * This means this app now again can not be enabled if the `max-version` is smaller than the current Nextcloud version.
+	 */
+	public function removeOverwriteNextcloudRequirement(string $appId): void {
+		$ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []);
+		$ignoreMaxApps = array_filter($ignoreMaxApps, fn (string $id) => $id !== $appId);
+		$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
 	}
 
 	public function loadApp(string $app): void {
@@ -573,7 +577,7 @@ public function enableApp(string $appId, bool $forceEnable = false): void {
 		$this->getAppPath($appId);
 
 		if ($forceEnable) {
-			$this->ignoreNextcloudRequirementForApp($appId);
+			$this->overwriteNextcloudRequirement($appId);
 		}
 
 		$this->installedAppsCache[$appId] = 'yes';
@@ -619,7 +623,7 @@ public function enableAppForGroups(string $appId, array $groups, bool $forceEnab
 		}
 
 		if ($forceEnable) {
-			$this->ignoreNextcloudRequirementForApp($appId);
+			$this->overwriteNextcloudRequirement($appId);
 		}
 
 		/** @var string[] $groupIds */
@@ -646,7 +650,7 @@ public function enableAppForGroups(string $appId, array $groups, bool $forceEnab
 	 * @param bool $automaticDisabled
 	 * @throws \Exception if app can't be disabled
 	 */
-	public function disableApp($appId, $automaticDisabled = false) {
+	public function disableApp($appId, $automaticDisabled = false): void {
 		if ($this->isAlwaysEnabled($appId)) {
 			throw new \Exception("$appId can't be disabled.");
 		}
@@ -706,7 +710,7 @@ public function getAppWebPath(string $appId): string {
 	/**
 	 * Clear the cached list of apps when enabling/disabling an app
 	 */
-	public function clearAppsCache() {
+	public function clearAppsCache(): void {
 		$this->appInfos = [];
 	}
 

diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
@@ -144,7 +144,7 @@ public function enableAppForGroups(string $appId, array $groups, bool $forceEnab
 	 * @param bool $automaticDisabled
 	 * @since 8.0.0
 	 */
-	public function disableApp($appId, $automaticDisabled = false);
+	public function disableApp($appId, $automaticDisabled = false): void;
 
 	/**
 	 * Get the directory for the given app.
@@ -185,7 +185,7 @@ public function getInstalledApps();
 	 * Clear the cached list of apps when enabling/disabling an app
 	 * @since 8.1.0
 	 */
-	public function clearAppsCache();
+	public function clearAppsCache(): void;
 
 	/**
 	 * @param string $appId
@@ -201,7 +201,7 @@ public function isShipped($appId);
 	 * @return bool
 	 *
 	 * This function walks through the Nextcloud directory and loads all apps
-	 * it can find. A directory contains an app if the file /appinfo/info.xml
+	 * it can find. A directory contains an app if the file `/appinfo/info.xml`
 	 * exists.
 	 *
 	 * if $types is set to non-empty array, only apps of those types will be loaded
@@ -271,7 +271,7 @@ public function getDefaultApps(): array;
 	/**
 	 * Set the global default apps with fallbacks
 	 *
-	 * @param string[] $appId
+	 * @param string[] $defaultApps
 	 * @throws \InvalidArgumentException If any of the apps is not installed
 	 * @since 28.0.0
 	 * @deprecated 31.0.0