fix!: Private apps are always auto enabled when updated (#33417)

* fix: Private apps are auto-enabled when updated regardless of any condition
RocketChat · Oct 17, 2024 · da9df9a · da9df9a
1 parent 8c07f76
commit da9df9a
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 4 deletions.
diff --git a/.changeset/selfish-experts-develop.md b/.changeset/selfish-experts-develop.md
@@ -0,0 +1,6 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/apps-engine": patch
+---
+
+Fixes issue with previously disabled private apps being auto enabled on update
diff --git a/apps/meteor/ee/server/apps/communication/websockets.ts b/apps/meteor/ee/server/apps/communication/websockets.ts
@@ -92,7 +92,12 @@ export class AppServerListener {
 
 		const appPackage = await this.orch.getAppSourceStorage()!.fetch(storageItem);
 
-		await this.orch.getManager()!.updateLocal(storageItem, appPackage);
+		const isEnabled = AppStatusUtils.isEnabled(storageItem.status);
+		if (isEnabled) {
+			await this.orch.getManager()!.updateAndStartupLocal(storageItem, appPackage);
+		} else {
+			await this.orch.getManager()!.updateAndInitializeLocal(storageItem, appPackage);
+		}
 
 		this.clientStreamer.emitWithoutBroadcast(AppEvents.APP_UPDATED, appId);
 	}

diff --git a/apps/meteor/server/services/apps-engine/service.ts b/apps/meteor/server/services/apps-engine/service.ts
@@ -62,7 +62,12 @@ export class AppsEngineService extends ServiceClassInternal implements IAppsEngi
 				return;
 			}
 
-			await Apps.self?.getManager()?.updateLocal(storageItem, appPackage);
+			const isEnabled = AppStatusUtils.isEnabled(storageItem.status);
+			if (isEnabled) {
+				await Apps.self?.getManager()?.updateAndStartupLocal(storageItem, appPackage);
+			} else {
+				await Apps.self?.getManager()?.updateAndInitializeLocal(storageItem, appPackage);
+			}
 		});
 
 		this.onEvent('apps.statusUpdate', async (appId: string, status: AppStatus): Promise<void> => {

diff --git a/apps/meteor/tests/e2e/apps/private-apps-upload.spec.ts b/apps/meteor/tests/e2e/apps/private-apps-upload.spec.ts
@@ -32,6 +32,56 @@ test.describe.serial('Private apps upload', () => {
 			await page.getByRole('button', { name: 'Agree' }).click();
 			await expect(poMarketplace.appStatusTag).toHaveText('Enabled');
 		});
+
+		test('expect to allow admin to update a enabled private app in EE, which should remain enabled', async ({ page }) => {
+			const fileChooserPromise = page.waitForEvent('filechooser');
+
+			await poMarketplace.btnUploadPrivateApp.click();
+			await expect(poMarketplace.btnInstallPrivateApp).toBeDisabled();
+
+			await poMarketplace.btnUploadPrivateAppFile.click();
+			const fileChooser = await fileChooserPromise;
+			await fileChooser.setFiles('./tests/e2e/fixtures/files/test-app_0.0.1.zip');
+
+			await expect(poMarketplace.btnInstallPrivateApp).toBeEnabled();
+			await poMarketplace.btnInstallPrivateApp.click();
+			await poMarketplace.btnConfirmAppUpdate.click();
+			await page.getByRole('button', { name: 'Agree' }).click();
+
+			await page.goto('/marketplace/private');
+			await poMarketplace.lastAppRow.click();
+			await expect(poMarketplace.appStatusTag).toHaveText('Enabled');
+		});
+
+		test('expect to allow disabling a recently installed private app in EE', async () => {
+			await poMarketplace.lastAppRow.click();
+			await expect(poMarketplace.appStatusTag).toHaveText('Enabled');
+			await poMarketplace.appMenu.click();
+			await expect(poMarketplace.btnDisableApp).toBeEnabled();
+			await poMarketplace.btnDisableApp.click();
+			await poMarketplace.btnConfirmAppUpdate.click();
+			await expect(poMarketplace.appStatusTag).toHaveText('Disabled');
+		});
+
+		test('expect to allow admin to update a disabled private app in EE, which should remain disabled', async ({ page }) => {
+			const fileChooserPromise = page.waitForEvent('filechooser');
+
+			await poMarketplace.btnUploadPrivateApp.click();
+			await expect(poMarketplace.btnInstallPrivateApp).toBeDisabled();
+
+			await poMarketplace.btnUploadPrivateAppFile.click();
+			const fileChooser = await fileChooserPromise;
+			await fileChooser.setFiles('./tests/e2e/fixtures/files/test-app_0.0.1.zip');
+
+			await expect(poMarketplace.btnInstallPrivateApp).toBeEnabled();
+			await poMarketplace.btnInstallPrivateApp.click();
+			await poMarketplace.btnConfirmAppUpdate.click();
+			await page.getByRole('button', { name: 'Agree' }).click();
+
+			await page.goto('/marketplace/private');
+			await poMarketplace.lastAppRow.click();
+			await expect(poMarketplace.appStatusTag).toHaveText('Disabled');
+		});
 	});
 
 	test.describe('Community Edition', () => {
@@ -66,5 +116,32 @@ test.describe.serial('Private apps upload', () => {
 			await poMarketplace.appMenu.click();
 			await expect(poMarketplace.btnEnableApp).toBeDisabled();
 		});
+
+		test('expect updated private app in CE to be kept as disabled', async ({ page }) => {
+			const fileChooserPromise = page.waitForEvent('filechooser');
+
+			await poMarketplace.btnUploadPrivateApp.click();
+			await expect(poMarketplace.btnConfirmAppUploadModal).toBeEnabled();
+			await poMarketplace.btnConfirmAppUploadModal.click();
+
+			await expect(poMarketplace.btnInstallPrivateApp).toBeDisabled();
+			await poMarketplace.btnUploadPrivateAppFile.click();
+			const fileChooser = await fileChooserPromise;
+			await fileChooser.setFiles('./tests/e2e/fixtures/files/test-app_0.0.1.zip');
+
+			await expect(poMarketplace.btnInstallPrivateApp).toBeEnabled();
+			await poMarketplace.btnInstallPrivateApp.click();
+
+			await expect(poMarketplace.confirmAppUploadModalTitle).toHaveText('Private apps limit reached');
+			await expect(poMarketplace.btnConfirmAppUploadModal).toBeEnabled();
+			await poMarketplace.btnConfirmAppUploadModal.click();
+
+			await poMarketplace.btnConfirmAppUpdate.click();
+
+			await page.getByRole('button', { name: 'Agree' }).click();
+			await page.goto('/marketplace/private');
+			await poMarketplace.lastAppRow.click();
+			await expect(poMarketplace.appStatusTag).toHaveText('Disabled');
+		});
 	});
 });
diff --git a/apps/meteor/tests/e2e/page-objects/marketplace.ts b/apps/meteor/tests/e2e/page-objects/marketplace.ts
@@ -42,4 +42,12 @@ export class Marketplace {
 	get btnEnableApp(): Locator {
 		return this.page.getByRole('menuitem', { name: 'Enable' });
 	}
+
+	get btnDisableApp(): Locator {
+		return this.page.getByRole('menuitem', { name: 'Disable' });
+	}
+
+	get btnConfirmAppUpdate(): Locator {
+		return this.page.locator('role=button[name="Yes"]');
+	}
 }
diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts
@@ -720,7 +720,12 @@ export class AppManager {
         aff.setApp(app);
 
         if (updateOptions.loadApp) {
-            await this.updateLocal(stored, app);
+            const shouldEnableApp = AppStatusUtils.isEnabled(old.status);
+            if (shouldEnableApp) {
+                await this.updateAndStartupLocal(stored, app);
+            } else {
+                await this.updateAndInitializeLocal(stored, app);
+            }
 
             await this.bridges
                 .getAppActivationBridge()
@@ -742,7 +747,7 @@ export class AppManager {
      * With an instance of a ProxiedApp, start it up and replace
      * the reference in the local app collection
      */
-    public async updateLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) {
+    async updateLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer): Promise<ProxiedApp> {
         const app = await (async () => {
             if (appPackageOrInstance instanceof Buffer) {
                 const parseResult = await this.getParser().unpackageApp(appPackageOrInstance);
@@ -761,10 +766,19 @@ export class AppManager {
         await this.purgeAppConfig(app, { keepScheduledJobs: true });
 
         this.apps.set(app.getID(), app);
+        return app;
+    }
 
+    public async updateAndStartupLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) {
+        const app = await this.updateLocal(stored, appPackageOrInstance);
         await this.runStartUpProcess(stored, app, false, true);
     }
 
+    public async updateAndInitializeLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) {
+        const app = await this.updateLocal(stored, appPackageOrInstance);
+        await this.initializeApp(stored, app, true, true);
+    }
+
     public getLanguageContent(): { [key: string]: object } {
         const langs: { [key: string]: object } = {};