fix(apps): prevent installation invalidation on app cron updates

.changeset/rotten-jars-occur.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/apps-engine': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes a bug that would cause apps to go into `invalid_installation_disabled` in some cases

packages/apps-engine/src/server/AppManager.ts

@@ -904,10 +904,15 @@ export class AppManager {
 				}
 
 				appStorageItem.marketplaceInfo[0].subscriptionInfo = appInfo.subscriptionInfo;
+				appStorageItem.signature = await this.getSignatureManager().signApp(appStorageItem);
 
-				return this.appMetadataStorage.updateMarketplaceInfo(appStorageItem._id, appStorageItem.marketplaceInfo);
+				return this.appMetadataStorage.updatePartialAndReturnDocument({
+					_id: appStorageItem._id,
+					marketplaceInfo: appStorageItem.marketplaceInfo,
+					signature: appStorageItem.signature,
+				});
 			}),
-		).catch();
+		).catch(() => {});
 
 		const queue = [] as Array<Promise<void>>;
 
@@ -928,7 +933,7 @@ export class AppManager {
 							return;
 						}
 
-						await this.purgeAppConfig(app);
+						await this.purgeAppConfig(app, { keepScheduledJobs: true });
 
 						return app.setStatus(AppStatus.INVALID_LICENSE_DISABLED);
 					})

packages/apps-engine/tests/server/AppManager.spec.ts

@@ -1,4 +1,4 @@
-import { Expect, SetupFixture, Teardown, Test } from 'alsatian';
+import { AsyncTest, Expect, SetupFixture, SpyOn, Teardown, Test } from 'alsatian';
 
 import { AppManager } from '../../src/server/AppManager';
 import { AppBridges } from '../../src/server/bridges';
@@ -14,7 +14,7 @@ import {
 	AppOutboundCommunicationProviderManager,
 } from '../../src/server/managers';
 import type { AppLogStorage, AppMetadataStorage, AppSourceStorage } from '../../src/server/storage';
-import { SimpleClass, TestInfastructureSetup } from '../test-data/utilities';
+import { SimpleClass, TestData, TestInfastructureSetup } from '../test-data/utilities';
 
 export class AppManagerTestFixture {
 	private testingInfastructure: TestInfastructureSetup;
@@ -121,4 +121,142 @@ export class AppManagerTestFixture {
 		Expect(manager.getVideoConfProviderManager() instanceof AppVideoConfProviderManager).toBe(true);
 		Expect(manager.getOutboundCommunicationProviderManager() instanceof AppOutboundCommunicationProviderManager).toBe(true);
 	}
+
+	@AsyncTest('Update Apps Marketplace Info - Apps without subscription info are skipped')
+	public async updateAppsMarketplaceInfoSkipsAppsWithoutSubscriptionInfo() {
+		const manager = new AppManager({
+			metadataStorage: this.testingInfastructure.getAppStorage(),
+			logStorage: this.testingInfastructure.getLogStorage(),
+			bridges: this.testingInfastructure.getAppBridges(),
+			sourceStorage: this.testingInfastructure.getSourceStorage(),
+		});
+
+		const appsOverview = TestData.getAppsOverview();
+		appsOverview[0].latest.subscriptionInfo = undefined; // No subscription info
+
+		// Mock the apps Map to return our mock app
+		(manager as any).apps = new Map([['test-app', TestData.getMockApp(TestData.getAppStorageItem(), manager)]]);
+
+		const updatePartialAndReturnDocumentSpy = SpyOn(manager.getStorage(), 'updatePartialAndReturnDocument');
+		updatePartialAndReturnDocumentSpy.andReturn(Promise.resolve());
+
+		// Should not throw and complete successfully
+		await manager.updateAppsMarketplaceInfo(appsOverview);
+
+		Expect(updatePartialAndReturnDocumentSpy).not.toHaveBeenCalled();
+	}
+
+	@AsyncTest('Update Apps Marketplace Info - Apps not found in manager are skipped')
+	public async updateAppsMarketplaceInfoSkipsAppsNotInManager() {
+		const manager = new AppManager({
+			metadataStorage: this.testingInfastructure.getAppStorage(),
+			logStorage: this.testingInfastructure.getLogStorage(),
+			bridges: this.testingInfastructure.getAppBridges(),
+			sourceStorage: this.testingInfastructure.getSourceStorage(),
+		});
+
+		const appsOverview = TestData.getAppsOverview();
+		appsOverview[0].latest.id = 'nonexistent-app'; // App not in manager
+
+		// Mock the apps Map to return our mock app
+		(manager as any).apps = new Map([['test-app', TestData.getMockApp(TestData.getAppStorageItem(), manager)]]);
+
+		const updatePartialAndReturnDocumentSpy = SpyOn(manager.getStorage(), 'updatePartialAndReturnDocument');
+		updatePartialAndReturnDocumentSpy.andReturn(Promise.resolve());
+
+		// Should not throw and complete successfully
+		await manager.updateAppsMarketplaceInfo(appsOverview);
+
+		Expect(updatePartialAndReturnDocumentSpy).not.toHaveBeenCalled();
+	}
+
+	@AsyncTest('Update Apps Marketplace Info - Apps with same license are skipped')
+	public async updateAppsMarketplaceInfoSkipsAppsWithSameLicense() {
+		const manager = new AppManager({
+			metadataStorage: this.testingInfastructure.getAppStorage(),
+			logStorage: this.testingInfastructure.getLogStorage(),
+			bridges: this.testingInfastructure.getAppBridges(),
+			sourceStorage: this.testingInfastructure.getSourceStorage(),
+		});
+
+		const sameLicenseData = 'same-license-data';
+		const existingSubscriptionInfo = TestData.getMarketplaceSubscriptionInfo({
+			license: { license: sameLicenseData, version: 1, expireDate: new Date('2023-01-01') },
+		});
+
+		const mockStorageItem = TestData.getAppStorageItem({
+			marketplaceInfo: [TestData.getMarketplaceInfo({ subscriptionInfo: existingSubscriptionInfo })],
+		});
+
+		const mockApp = TestData.getMockApp(mockStorageItem, manager);
+
+		// Mock the apps Map to return our mock app
+		(manager as any).apps = new Map([['test-app', mockApp]]);
+
+		const appsOverview = TestData.getAppsOverview(
+			TestData.getMarketplaceSubscriptionInfo({
+				license: { license: sameLicenseData, version: 1, expireDate: new Date('2023-01-01') },
+			}),
+		);
+
+		const updatePartialAndReturnDocumentSpy = SpyOn(manager.getStorage(), 'updatePartialAndReturnDocument');
+		updatePartialAndReturnDocumentSpy.andReturn(Promise.resolve());
+
+		// Should not throw and complete successfully
+		await manager.updateAppsMarketplaceInfo(appsOverview);
+
+		// Verify the subscription info was not updated (should remain the same)
+		Expect(mockStorageItem.marketplaceInfo[0].subscriptionInfo.seats).toBe(10); // Original value
+		Expect(updatePartialAndReturnDocumentSpy).not.toHaveBeenCalled();
+	}
+
+	@AsyncTest('Update Apps Marketplace Info - Subscription info is updated and app is signed')
+	public async updateAppsMarketplaceInfoUpdatesSubscriptionAndSignsApp() {
+		const manager = new AppManager({
+			metadataStorage: this.testingInfastructure.getAppStorage(),
+			logStorage: this.testingInfastructure.getLogStorage(),
+			bridges: this.testingInfastructure.getAppBridges(),
+			sourceStorage: this.testingInfastructure.getSourceStorage(),
+		});
+
+		const existingSubscriptionInfo = TestData.getMarketplaceSubscriptionInfo({
+			license: { license: 'old-license-data', version: 1, expireDate: new Date('2023-01-01') },
+		});
+
+		const newSubscriptionInfo = TestData.getMarketplaceSubscriptionInfo({
+			seats: 20,
+			maxSeats: 200,
+			startDate: '2023-02-01',
+			periodEnd: '2024-01-31',
+			license: { license: 'new-license-data', version: 1, expireDate: new Date('2026-01-01') },
+		});
+
+		const mockStorageItem = TestData.getAppStorageItem({
+			marketplaceInfo: [TestData.getMarketplaceInfo({ subscriptionInfo: existingSubscriptionInfo })],
+		});
+
+		const mockApp = TestData.getMockApp(mockStorageItem, manager);
+
+		// eslint-disable-next-line no-return-assign
+		SpyOn(manager.getSignatureManager(), 'signApp').andReturn(Promise.resolve('signed-app-data'));
+		SpyOn(mockApp, 'validateLicense').andReturn(Promise.resolve());
+
+		const updatePartialAndReturnDocumentSpy = SpyOn(manager.getStorage(), 'updatePartialAndReturnDocument');
+		updatePartialAndReturnDocumentSpy.andReturn(Promise.resolve(mockStorageItem));
+
+		// Mock the apps Map and dependencies
+		(manager as any).apps = new Map([['test-app', mockApp]]);
+
+		const appsOverview = TestData.getAppsOverview(newSubscriptionInfo);
+
+		await manager.updateAppsMarketplaceInfo(appsOverview);
+
+		const expectedStorageItem = mockApp.getStorageItem();
+
+		// Verify the subscription info was updated
+		Expect(expectedStorageItem.marketplaceInfo[0].subscriptionInfo.seats).toBe(20);
+		Expect(expectedStorageItem.marketplaceInfo[0].subscriptionInfo.license.license).toBe('new-license-data');
+		Expect(expectedStorageItem.signature).toBe('signed-app-data');
+		Expect(updatePartialAndReturnDocumentSpy).toHaveBeenCalled().exactly(1).times;
+	}
 }

packages/apps-engine/tests/server/managers/AppApiManager.spec.ts

@@ -16,7 +16,7 @@ import type {
 import { AppAccessorManager, AppApiManager } from '../../../src/server/managers';
 import { AppApi } from '../../../src/server/managers/AppApi';
 import type { UIActionButtonManager } from '../../../src/server/managers/UIActionButtonManager';
-import type { AppLogStorage } from '../../../src/server/storage';
+import type { AppLogStorage, IAppStorageItem } from '../../../src/server/storage';
 import { TestsAppBridges } from '../../test-data/bridges/appBridges';
 import { TestsAppLogStorage } from '../../test-data/storage/logStorage';
 import { TestData } from '../../test-data/utilities';
@@ -38,7 +38,7 @@ export class AppApiManagerTestFixture {
 	public setupFixture() {
 		this.mockBridges = new TestsAppBridges();
 
-		this.mockApp = TestData.getMockApp({ id: 'testing', name: 'TestApp' }, this.mockManager);
+		this.mockApp = TestData.getMockApp({ info: { id: 'testing', name: 'TestApp' } } as IAppStorageItem, this.mockManager);
 
 		const bri = this.mockBridges;
 		const app = this.mockApp;

packages/apps-engine/tests/server/managers/AppSlashCommand.spec.ts

@@ -1,5 +1,6 @@
 import { Expect, SetupFixture, Test } from 'alsatian';
 
+import type { IAppStorageItem } from '../../../server/storage';
 import type { ISlashCommand } from '../../../src/definition/slashcommands';
 import type { AppManager } from '../../../src/server/AppManager';
 import type { ProxiedApp } from '../../../src/server/ProxiedApp';
@@ -11,7 +12,7 @@ export class AppSlashCommandRegistrationTestFixture {
 
 	@SetupFixture
 	public setupFixture() {
-		this.mockApp = TestData.getMockApp({ id: 'test', name: 'TestApp' }, {} as AppManager);
+		this.mockApp = TestData.getMockApp({ info: { id: 'test', name: 'TestApp' } } as IAppStorageItem, {} as AppManager);
 	}
 
 	@Test()

packages/apps-engine/tests/server/managers/AppSlashCommandManager.spec.ts

@@ -17,7 +17,7 @@ import { AppAccessorManager, AppSlashCommandManager } from '../../../src/server/
 import { AppSlashCommand } from '../../../src/server/managers/AppSlashCommand';
 import type { UIActionButtonManager } from '../../../src/server/managers/UIActionButtonManager';
 import { Room } from '../../../src/server/rooms/Room';
-import type { AppLogStorage } from '../../../src/server/storage';
+import type { AppLogStorage, IAppStorageItem } from '../../../src/server/storage';
 import { TestsAppBridges } from '../../test-data/bridges/appBridges';
 import { TestsAppLogStorage } from '../../test-data/storage/logStorage';
 import { TestData } from '../../test-data/utilities';
@@ -39,7 +39,7 @@ export class AppSlashCommandManagerTestFixture {
 	public setupFixture() {
 		this.mockBridges = new TestsAppBridges();
 
-		this.mockApp = TestData.getMockApp({ id: 'testing', name: 'TestApp' }, this.mockManager);
+		this.mockApp = TestData.getMockApp({ info: { id: 'testing', name: 'TestApp' } } as IAppStorageItem, this.mockManager);
 
 		const bri = this.mockBridges;
 		const app = this.mockApp;

packages/apps-engine/tests/server/managers/AppVideoConfProviderManager.spec.ts

@@ -8,7 +8,7 @@ import type { AppApiManager, AppExternalComponentManager, AppSchedulerManager, A
 import { AppAccessorManager, AppVideoConfProviderManager } from '../../../src/server/managers';
 import { AppVideoConfProvider } from '../../../src/server/managers/AppVideoConfProvider';
 import type { UIActionButtonManager } from '../../../src/server/managers/UIActionButtonManager';
-import type { AppLogStorage } from '../../../src/server/storage';
+import type { AppLogStorage, IAppStorageItem } from '../../../src/server/storage';
 import { TestsAppBridges } from '../../test-data/bridges/appBridges';
 import { TestsAppLogStorage } from '../../test-data/storage/logStorage';
 import { TestData } from '../../test-data/utilities';
@@ -28,7 +28,7 @@ export class AppVideoConfProviderManagerTestFixture {
 	public setupFixture() {
 		this.mockBridges = new TestsAppBridges();
 
-		this.mockApp = TestData.getMockApp({ id: 'testing', name: 'testing' }, this.mockManager);
+		this.mockApp = TestData.getMockApp({ info: { id: 'testing', name: 'testing' } } as IAppStorageItem, this.mockManager);
 
 		const bri = this.mockBridges;
 		const app = this.mockApp;

packages/apps-engine/tests/test-data/storage/storage.ts

@@ -1,3 +1,7 @@
+import type { AppStatus } from '../../../src/definition/AppStatus';
+import type { IAppInfo } from '../../../src/definition/metadata';
+import type { ISetting } from '../../../src/definition/settings';
+import type { IMarketplaceInfo } from '../../../src/server/marketplace';
 import type { IAppStorageItem } from '../../../src/server/storage';
 import { AppMetadataStorage } from '../../../src/server/storage';
 
@@ -82,20 +86,6 @@ export class TestsAppStorage extends AppMetadataStorage {
 		});
 	}
 
-	public update(item: IAppStorageItem): Promise<IAppStorageItem> {
-		return new Promise((resolve, reject) => {
-			this.db.update({ id: item.id }, item, {}, (err, _numOfUpdated: number) => {
-				if (err) {
-					reject(err);
-				} else {
-					this.retrieveOne(item.id)
-						.then((updated: IAppStorageItem) => resolve(updated))
-						.catch((err2: Error) => reject(err2));
-				}
-			});
-		});
-	}
-
 	public remove(id: string): Promise<{ success: boolean }> {
 		return new Promise((resolve, reject) => {
 			this.db.remove({ id }, (err) => {
@@ -107,4 +97,27 @@ export class TestsAppStorage extends AppMetadataStorage {
 			});
 		});
 	}
+
+	public updatePartialAndReturnDocument(
+		item: Partial<IAppStorageItem>,
+		options?: { unsetPermissionsGranted?: boolean },
+	): Promise<IAppStorageItem> {
+		throw new Error('Method not implemented.');
+	}
+
+	public updateStatus(_id: string, status: AppStatus): Promise<boolean> {
+		throw new Error('Method not implemented.');
+	}
+
+	public updateSetting(_id: string, setting: ISetting): Promise<boolean> {
+		throw new Error('Method not implemented.');
+	}
+
+	public updateAppInfo(_id: string, info: IAppInfo): Promise<boolean> {
+		throw new Error('Method not implemented.');
+	}
+
+	public updateMarketplaceInfo(_id: string, marketplaceInfo: IMarketplaceInfo[]): Promise<boolean> {
+		throw new Error('Method not implemented.');
+	}
 }