diff --git a/.changeset/rotten-jars-occur.md b/.changeset/rotten-jars-occur.md new file mode 100644 index 0000000000000..e11d510d2614c --- /dev/null +++ b/.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 diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 1a0480c54c144..173dfc18fbd76 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -909,10 +909,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>; @@ -933,7 +938,7 @@ export class AppManager { return; } - await this.purgeAppConfig(app); + await this.purgeAppConfig(app, { keepScheduledJobs: true }); return app.setStatus(AppStatus.INVALID_LICENSE_DISABLED); }) diff --git a/packages/apps-engine/tests/server/AppManager.spec.ts b/packages/apps-engine/tests/server/AppManager.spec.ts index 1f6ef3e4a18f2..b31900cb76301 100644 --- a/packages/apps-engine/tests/server/AppManager.spec.ts +++ b/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; + } } diff --git a/packages/apps-engine/tests/server/managers/AppApiManager.spec.ts b/packages/apps-engine/tests/server/managers/AppApiManager.spec.ts index ed683a9c6931a..7fb250b154e07 100644 --- a/packages/apps-engine/tests/server/managers/AppApiManager.spec.ts +++ b/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; diff --git a/packages/apps-engine/tests/server/managers/AppSlashCommand.spec.ts b/packages/apps-engine/tests/server/managers/AppSlashCommand.spec.ts index f6eda56c6fb02..7e4ed81caeeb8 100644 --- a/packages/apps-engine/tests/server/managers/AppSlashCommand.spec.ts +++ b/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() diff --git a/packages/apps-engine/tests/server/managers/AppSlashCommandManager.spec.ts b/packages/apps-engine/tests/server/managers/AppSlashCommandManager.spec.ts index 204e31e21fee1..f8e648625b765 100644 --- a/packages/apps-engine/tests/server/managers/AppSlashCommandManager.spec.ts +++ b/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; diff --git a/packages/apps-engine/tests/server/managers/AppVideoConfProviderManager.spec.ts b/packages/apps-engine/tests/server/managers/AppVideoConfProviderManager.spec.ts index 2ec03c61119a4..b78bca29888d8 100644 --- a/packages/apps-engine/tests/server/managers/AppVideoConfProviderManager.spec.ts +++ b/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; diff --git a/packages/apps-engine/tests/test-data/storage/storage.ts b/packages/apps-engine/tests/test-data/storage/storage.ts index 8412e67c65e24..dbab953c6487e 100644 --- a/packages/apps-engine/tests/test-data/storage/storage.ts +++ b/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 { - 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, + options?: { unsetPermissionsGranted?: boolean }, + ): Promise { + throw new Error('Method not implemented.'); + } + + public updateStatus(_id: string, status: AppStatus): Promise { + throw new Error('Method not implemented.'); + } + + public updateSetting(_id: string, setting: ISetting): Promise { + throw new Error('Method not implemented.'); + } + + public updateAppInfo(_id: string, info: IAppInfo): Promise { + throw new Error('Method not implemented.'); + } + + public updateMarketplaceInfo(_id: string, marketplaceInfo: IMarketplaceInfo[]): Promise { + throw new Error('Method not implemented.'); + } } diff --git a/packages/apps-engine/tests/test-data/utilities.ts b/packages/apps-engine/tests/test-data/utilities.ts index 049d346ed25a4..9cd3135ed061c 100644 --- a/packages/apps-engine/tests/test-data/utilities.ts +++ b/packages/apps-engine/tests/test-data/utilities.ts @@ -51,8 +51,13 @@ import type { } from '../../src/server/managers'; import type { AppRuntimeManager } from '../../src/server/managers/AppRuntimeManager'; import type { UIActionButtonManager } from '../../src/server/managers/UIActionButtonManager'; +import type { IMarketplaceInfo, IMarketplaceSubscriptionInfo } from '../../src/server/marketplace'; +import { MarketplacePurchaseType } from '../../src/server/marketplace/MarketplacePurchaseType'; +import { MarketplaceSubscriptionStatus } from '../../src/server/marketplace/MarketplaceSubscriptionStatus'; +import { MarketplaceSubscriptionType } from '../../src/server/marketplace/MarketplaceSubscriptionType'; import type { IRuntimeController } from '../../src/server/runtime/IRuntimeController'; import type { AppLogStorage, AppMetadataStorage, AppSourceStorage, IAppStorageItem } from '../../src/server/storage'; +import { AppInstallationSource } from '../../src/server/storage/IAppStorageItem'; export class TestInfastructureSetup { private appStorage: TestsAppStorage; @@ -100,7 +105,9 @@ export class TestInfastructureSetup { return {} as AppExternalComponentManager; }, getOneById(appId: string): ProxiedApp { - return appId === 'failMePlease' ? undefined : TestData.getMockApp({ id: appId, name: 'testing' }, this); + return appId === 'failMePlease' + ? undefined + : TestData.getMockApp({ info: { id: appId, name: 'testing' } } as IAppStorageItem, this); }, getLogStorage(): AppLogStorage { return new TestsAppLogStorage(); @@ -601,13 +608,100 @@ export class TestData { return mock; } - public static getMockApp({ id, name }: { id: string; name: string }, manager: AppManager): ProxiedApp { + public static getMockApp(storageItem: Partial, manager: AppManager): ProxiedApp { + const { id, name } = storageItem.info || { id: 'test-app', name: 'Test App' }; + return new ProxiedApp( manager, - { id, status: AppStatus.AUTO_ENABLED, info: { id, name } } as IAppStorageItem, + { id, status: AppStatus.AUTO_ENABLED, info: { id, name }, ...storageItem } as IAppStorageItem, TestData.getMockRuntimeController(id), ); } + + public static getMarketplaceSubscriptionInfo(overrides: Partial = {}): IMarketplaceSubscriptionInfo { + return { + seats: 10, + maxSeats: 100, + startDate: '2023-01-01', + periodEnd: '2023-12-31', + isSubscripbedViaBundle: false, + typeOf: MarketplaceSubscriptionType.SubscriptionTypeApp, + status: MarketplaceSubscriptionStatus.PurchaseSubscriptionStatusActive, + license: { + license: 'encrypted-license-data', + version: 1, + expireDate: new Date('2023-01-01'), + }, + ...overrides, + }; + } + + public static getMarketplaceInfo(overrides: Partial = {}): IMarketplaceInfo { + return { + id: 'test-app', + name: 'Test App', + nameSlug: 'test-app', + version: '1.0.0', + description: 'Test app', + author: { name: 'Test Author', support: 'https://test.com', homepage: 'https://test.com' }, + permissions: [], + requiredApiVersion: '1.0.0', + classFile: 'main.js', + iconFile: 'icon.png', + implements: [], + categories: [], + status: 'active', + isVisible: true, + isPurchased: false, + isSubscribed: false, + isBundled: false, + createdDate: '2023-01-01', + modifiedDate: '2023-01-01', + price: 0, + purchaseType: MarketplacePurchaseType.PurchaseTypeSubscription, + subscriptionInfo: TestData.getMarketplaceSubscriptionInfo(), + ...overrides, + }; + } + + public static getAppStorageItem(overrides: Partial = {}): IAppStorageItem { + return { + id: 'test-app', + status: AppStatus.AUTO_ENABLED, + info: { + id: 'test-app', + name: 'Test App', + nameSlug: 'test-app', + version: '1.0.0', + description: 'Test app', + author: { name: 'Test Author', support: 'https://test.com', homepage: 'https://test.com' }, + permissions: [], + requiredApiVersion: '1.0.0', + classFile: 'main.js', + iconFile: 'icon.png', + implements: [], + }, + marketplaceInfo: [TestData.getMarketplaceInfo()], + createdAt: new Date(), + updatedAt: new Date(), + installationSource: AppInstallationSource.MARKETPLACE, + languageContent: {}, + settings: {}, + implemented: {}, + signature: 'default-signature', + ...overrides, + }; + } + + public static getAppsOverview(subscriptionInfo?: IMarketplaceSubscriptionInfo): Array<{ latest: IMarketplaceInfo }> { + return [ + { + latest: TestData.getMarketplaceInfo({ + subscriptionInfo: subscriptionInfo || TestData.getMarketplaceSubscriptionInfo(), + }), + }, + ]; + } } export class SimpleClass {