From 9f7d4de647f5d9800d1439f9afaa20c1b3cd2919 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 3 Oct 2024 11:44:14 -0300 Subject: [PATCH 01/25] Change license types to accept any string as a module name --- .../core-typings/src/license/ILicenseV3.ts | 14 ++++-- .../src/license/LicenseBehavior.ts | 5 +- .../core-typings/src/license/LicenseInfo.ts | 3 +- .../core-typings/src/license/LicenseLimit.ts | 3 +- .../core-typings/src/license/LicenseModule.ts | 47 ++++++++++--------- .../core-typings/src/license/LicensePeriod.ts | 3 +- packages/core-typings/src/license/events.ts | 2 +- 7 files changed, 42 insertions(+), 35 deletions(-) diff --git a/packages/core-typings/src/license/ILicenseV3.ts b/packages/core-typings/src/license/ILicenseV3.ts index d99f80c71bfc..612eb34d2b28 100644 --- a/packages/core-typings/src/license/ILicenseV3.ts +++ b/packages/core-typings/src/license/ILicenseV3.ts @@ -3,6 +3,16 @@ import type { LicenseLimit } from './LicenseLimit'; import type { LicenseModule } from './LicenseModule'; import type { LicensePeriod, Timestamp } from './LicensePeriod'; +export type GrantedModules = ( + | { + module: LicenseModule; + } + | { + module: string; + external: boolean; + } +)[]; + export interface ILicenseV3 { version: '3.0'; information: { @@ -48,9 +58,7 @@ export interface ILicenseV3 { allowedStaleInDays?: number; }; }; - grantedModules: { - module: LicenseModule; - }[]; + grantedModules: GrantedModules; limits: { activeUsers?: LicenseLimit[]; guestUsers?: LicenseLimit[]; diff --git a/packages/core-typings/src/license/LicenseBehavior.ts b/packages/core-typings/src/license/LicenseBehavior.ts index ac2249233ab5..915f4e116073 100644 --- a/packages/core-typings/src/license/LicenseBehavior.ts +++ b/packages/core-typings/src/license/LicenseBehavior.ts @@ -1,5 +1,4 @@ import type { LicenseLimitKind } from './ILicenseV3'; -import type { LicenseModule } from './LicenseModule'; export type LicenseBehavior = | 'invalidate_license' @@ -12,12 +11,12 @@ export type LicenseBehavior = export type BehaviorWithContext = | { behavior: LicenseBehavior; - modules?: LicenseModule[]; + modules?: string[]; reason: 'limit'; limit?: LicenseLimitKind; } | { behavior: LicenseBehavior; - modules?: LicenseModule[]; + modules?: string[]; reason: 'period' | 'url'; }; diff --git a/packages/core-typings/src/license/LicenseInfo.ts b/packages/core-typings/src/license/LicenseInfo.ts index 019d1b9e1ca0..85b67dc652eb 100644 --- a/packages/core-typings/src/license/LicenseInfo.ts +++ b/packages/core-typings/src/license/LicenseInfo.ts @@ -1,10 +1,9 @@ import type { ILicenseTag } from './ILicenseTag'; import type { ILicenseV3, LicenseLimitKind } from './ILicenseV3'; -import type { LicenseModule } from './LicenseModule'; export type LicenseInfo = { license?: ILicenseV3; - activeModules: LicenseModule[]; + activeModules: string[]; preventedActions: Record; limits: Record; tags: ILicenseTag[]; diff --git a/packages/core-typings/src/license/LicenseLimit.ts b/packages/core-typings/src/license/LicenseLimit.ts index 40e5a62f597a..76ff615f51b4 100644 --- a/packages/core-typings/src/license/LicenseLimit.ts +++ b/packages/core-typings/src/license/LicenseLimit.ts @@ -1,7 +1,6 @@ import type { LicenseBehavior } from './LicenseBehavior'; -import type { LicenseModule } from './LicenseModule'; export type LicenseLimit = { max: number; behavior: T; -} & (T extends 'disable_modules' ? { behavior: T; modules: LicenseModule[] } : { behavior: T }); +} & (T extends 'disable_modules' ? { behavior: T; modules: string[] } : { behavior: T }); diff --git a/packages/core-typings/src/license/LicenseModule.ts b/packages/core-typings/src/license/LicenseModule.ts index c29ea45f0900..c5e7495d2b2d 100644 --- a/packages/core-typings/src/license/LicenseModule.ts +++ b/packages/core-typings/src/license/LicenseModule.ts @@ -1,22 +1,25 @@ -export type LicenseModule = - | 'auditing' - | 'canned-responses' - | 'ldap-enterprise' - | 'livechat-enterprise' - | 'voip-enterprise' - | 'omnichannel-mobile-enterprise' - | 'engagement-dashboard' - | 'push-privacy' - | 'scalability' - | 'teams-mention' - | 'saml-enterprise' - | 'oauth-enterprise' - | 'device-management' - | 'federation' - | 'videoconference-enterprise' - | 'message-read-receipt' - | 'outlook-calendar' - | 'hide-watermark' - | 'custom-roles' - | 'accessibility-certification' - | 'unlimited-presence'; +export const CoreModules = [ + 'auditing', + 'canned-responses', + 'ldap-enterprise', + 'livechat-enterprise', + 'voip-enterprise', + 'omnichannel-mobile-enterprise', + 'engagement-dashboard', + 'push-privacy', + 'scalability', + 'teams-mention', + 'saml-enterprise', + 'oauth-enterprise', + 'device-management', + 'federation', + 'videoconference-enterprise', + 'message-read-receipt', + 'outlook-calendar', + 'hide-watermark', + 'custom-roles', + 'accessibility-certification', + 'unlimited-presence', +] as const; + +export type LicenseModule = (typeof CoreModules)[number]; diff --git a/packages/core-typings/src/license/LicensePeriod.ts b/packages/core-typings/src/license/LicensePeriod.ts index d9bae6198fde..b7258e4ef6bf 100644 --- a/packages/core-typings/src/license/LicensePeriod.ts +++ b/packages/core-typings/src/license/LicensePeriod.ts @@ -1,5 +1,4 @@ import type { LicenseBehavior } from './LicenseBehavior'; -import type { LicenseModule } from './LicenseModule'; export type Timestamp = string; @@ -8,6 +7,6 @@ export type LicensePeriod = { validUntil?: Timestamp; invalidBehavior: LicenseBehavior; } & ({ validFrom: Timestamp } | { validUntil: Timestamp }) & - ({ invalidBehavior: 'disable_modules'; modules: LicenseModule[] } | { invalidBehavior: Exclude }); + ({ invalidBehavior: 'disable_modules'; modules: string[] } | { invalidBehavior: Exclude }); export type LicensePeriodBehavior = Exclude; diff --git a/packages/core-typings/src/license/events.ts b/packages/core-typings/src/license/events.ts index 2156c298849c..7045262c055a 100644 --- a/packages/core-typings/src/license/events.ts +++ b/packages/core-typings/src/license/events.ts @@ -19,6 +19,6 @@ export type LicenseEvents = ModuleValidation & removed: undefined; validate: undefined; invalidate: undefined; - module: { module: LicenseModule; valid: boolean }; + module: { module: string; valid: boolean }; sync: undefined; }; From f11daa76d94c6c5c3e32c2fc81cbca0f20dbd062 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 3 Oct 2024 14:43:43 -0300 Subject: [PATCH 02/25] Change references to modules in the license package to accept any string --- ee/packages/license/src/events/emitter.ts | 20 +++++++++++-- ee/packages/license/src/events/listeners.ts | 2 +- ee/packages/license/src/license.ts | 3 +- ee/packages/license/src/modules.ts | 10 +++---- ee/packages/license/src/v2/bundles.ts | 30 +++---------------- .../src/validation/getModulesToDisable.ts | 4 +-- 6 files changed, 29 insertions(+), 40 deletions(-) diff --git a/ee/packages/license/src/events/emitter.ts b/ee/packages/license/src/events/emitter.ts index b4406abaa81b..cbf88c0b4677 100644 --- a/ee/packages/license/src/events/emitter.ts +++ b/ee/packages/license/src/events/emitter.ts @@ -1,14 +1,23 @@ -import type { BehaviorWithContext, LicenseModule } from '@rocket.chat/core-typings'; +import { CoreModules, type BehaviorWithContext, type LicenseModule } from '@rocket.chat/core-typings'; import type { LicenseManager } from '../license'; import { logger } from '../logger'; -export function moduleValidated(this: LicenseManager, module: LicenseModule) { +function isLicenseModule(module: string): module is LicenseModule { + return CoreModules.includes(module as LicenseModule); +} + +export function moduleValidated(this: LicenseManager, module: string) { try { this.emit('module', { module, valid: true }); } catch (error) { logger.error({ msg: `Error running module (valid: true) event: ${module}`, error }); } + + if (!isLicenseModule(module)) { + return; + } + try { this.emit(`valid:${module}`); } catch (error) { @@ -16,12 +25,17 @@ export function moduleValidated(this: LicenseManager, module: LicenseModule) { } } -export function moduleRemoved(this: LicenseManager, module: LicenseModule) { +export function moduleRemoved(this: LicenseManager, module: string) { try { this.emit('module', { module, valid: false }); } catch (error) { logger.error({ msg: `Error running module (valid: false) event: ${module}`, error }); } + + if (!isLicenseModule(module)) { + return; + } + try { this.emit(`invalid:${module}`); } catch (error) { diff --git a/ee/packages/license/src/events/listeners.ts b/ee/packages/license/src/events/listeners.ts index 7b7eaf0baed9..c327789db467 100644 --- a/ee/packages/license/src/events/listeners.ts +++ b/ee/packages/license/src/events/listeners.ts @@ -83,7 +83,7 @@ export function onToggledFeature( }; } -export function onModule(this: LicenseManager, cb: (data: { module: LicenseModule; valid: boolean }) => void) { +export function onModule(this: LicenseManager, cb: (data: { module: string; valid: boolean }) => void) { this.on('module', cb); } diff --git a/ee/packages/license/src/license.ts b/ee/packages/license/src/license.ts index a8d8f1bca510..f3c1c1f4f49b 100644 --- a/ee/packages/license/src/license.ts +++ b/ee/packages/license/src/license.ts @@ -7,7 +7,6 @@ import type { BehaviorWithContext, LicenseBehavior, LicenseInfo, - LicenseModule, LicenseValidationOptions, LimitContext, } from '@rocket.chat/core-typings'; @@ -43,7 +42,7 @@ export class LicenseManager extends Emitter { tags = new Set(); - modules = new Set(); + modules = new Set(); private workspaceUrl: string | undefined; diff --git a/ee/packages/license/src/modules.ts b/ee/packages/license/src/modules.ts index 7a6f41c07ec1..faa167c1bfc3 100644 --- a/ee/packages/license/src/modules.ts +++ b/ee/packages/license/src/modules.ts @@ -1,16 +1,14 @@ -import type { LicenseModule } from '@rocket.chat/core-typings'; - import { moduleRemoved, moduleValidated } from './events/emitter'; import type { LicenseManager } from './license'; -export function notifyValidatedModules(this: LicenseManager, licenseModules: LicenseModule[]) { +export function notifyValidatedModules(this: LicenseManager, licenseModules: string[]) { licenseModules.forEach((module) => { this.modules.add(module); moduleValidated.call(this, module); }); } -export function notifyInvalidatedModules(this: LicenseManager, licenseModules: LicenseModule[]) { +export function notifyInvalidatedModules(this: LicenseManager, licenseModules: string[]) { licenseModules.forEach((module) => { moduleRemoved.call(this, module); this.modules.delete(module); @@ -26,11 +24,11 @@ export function getModules(this: LicenseManager) { return [...this.modules]; } -export function hasModule(this: LicenseManager, module: LicenseModule) { +export function hasModule(this: LicenseManager, module: string) { return this.modules.has(module); } -export function replaceModules(this: LicenseManager, newModules: LicenseModule[]): boolean { +export function replaceModules(this: LicenseManager, newModules: string[]): boolean { let anyChange = false; for (const moduleName of newModules) { if (this.modules.has(moduleName)) { diff --git a/ee/packages/license/src/v2/bundles.ts b/ee/packages/license/src/v2/bundles.ts index 0ef4296d8223..518fbe41e1b3 100644 --- a/ee/packages/license/src/v2/bundles.ts +++ b/ee/packages/license/src/v2/bundles.ts @@ -1,33 +1,11 @@ -import type { LicenseModule } from '@rocket.chat/core-typings'; +import { CoreModules, type LicenseModule } from '@rocket.chat/core-typings'; interface IBundle { - [key: string]: LicenseModule[]; + [key: string]: readonly LicenseModule[]; } const bundles: IBundle = { - enterprise: [ - 'auditing', - 'canned-responses', - 'ldap-enterprise', - 'livechat-enterprise', - 'voip-enterprise', - 'omnichannel-mobile-enterprise', - 'engagement-dashboard', - 'push-privacy', - 'scalability', - 'saml-enterprise', - 'oauth-enterprise', - 'device-management', - 'federation', - 'videoconference-enterprise', - 'message-read-receipt', - 'outlook-calendar', - 'teams-mention', - 'hide-watermark', - 'custom-roles', - 'accessibility-certification', - 'unlimited-presence', - ], + enterprise: CoreModules, pro: [], }; @@ -53,7 +31,7 @@ export function isBundle(moduleName: string): boolean { return true; } -export function getBundleModules(moduleName: string): string[] { +export function getBundleModules(moduleName: string): readonly string[] { if (moduleName === '*') { return Object.keys(bundles).reduce((modules, bundle) => modules.concat(bundles[bundle]), []); } diff --git a/ee/packages/license/src/validation/getModulesToDisable.ts b/ee/packages/license/src/validation/getModulesToDisable.ts index cdab814db026..7ce8f98abe79 100644 --- a/ee/packages/license/src/validation/getModulesToDisable.ts +++ b/ee/packages/license/src/validation/getModulesToDisable.ts @@ -1,9 +1,9 @@ -import type { BehaviorWithContext, LicenseBehavior, LicenseModule } from '@rocket.chat/core-typings'; +import type { BehaviorWithContext, LicenseBehavior } from '@rocket.chat/core-typings'; const filterValidationResult = (result: BehaviorWithContext[], expectedBehavior: LicenseBehavior) => result.filter(({ behavior }) => behavior === expectedBehavior) as BehaviorWithContext[]; -export const getModulesToDisable = (validationResult: BehaviorWithContext[]): LicenseModule[] => { +export const getModulesToDisable = (validationResult: BehaviorWithContext[]): string[] => { return [ ...new Set([ ...filterValidationResult(validationResult, 'disable_modules') From fadfd4e46d79d488babcb62ebef5778e009e6218 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 3 Oct 2024 15:08:18 -0300 Subject: [PATCH 03/25] Include test add-on in license unit test --- .../license/__tests__/setLicense.spec.ts | 4 ++- .../license/src/MockedLicenseBuilder.ts | 25 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/ee/packages/license/__tests__/setLicense.spec.ts b/ee/packages/license/__tests__/setLicense.spec.ts index 1caf8cafa2cd..41785c756cc0 100644 --- a/ee/packages/license/__tests__/setLicense.spec.ts +++ b/ee/packages/license/__tests__/setLicense.spec.ts @@ -140,16 +140,18 @@ describe('License set license procedures', () => { const mocked = new MockedLicenseBuilder(); const oldToken = await mocked.sign(); - const newToken = await mocked.withGratedModules(['livechat-enterprise']).sign(); + const newToken = await mocked.withGratedModules(['livechat-enterprise', 'chat.rocket.test-addon']).sign(); await expect(license.setLicense(oldToken)).resolves.toBe(true); expect(license.hasValidLicense()).toBe(true); expect(license.hasModule('livechat-enterprise')).toBe(false); + expect(license.hasModule('chat.rocket.test-addon')).toBe(false); await expect(license.setLicense(newToken)).resolves.toBe(true); expect(license.hasValidLicense()).toBe(true); expect(license.hasModule('livechat-enterprise')).toBe(true); + expect(license.hasModule('chat.rocket.test-addon')).toBe(true); }); it('should call a validated event after set a valid license', async () => { diff --git a/ee/packages/license/src/MockedLicenseBuilder.ts b/ee/packages/license/src/MockedLicenseBuilder.ts index 4c3cab7b660f..1b83900993aa 100644 --- a/ee/packages/license/src/MockedLicenseBuilder.ts +++ b/ee/packages/license/src/MockedLicenseBuilder.ts @@ -1,4 +1,13 @@ -import type { ILicenseTag, ILicenseV3, LicenseLimit, LicenseModule, LicensePeriod, Timestamp } from '@rocket.chat/core-typings'; +import { + CoreModules, + type GrantedModules, + type ILicenseTag, + type ILicenseV3, + type LicenseLimit, + type LicenseModule, + type LicensePeriod, + type Timestamp, +} from '@rocket.chat/core-typings'; import { encrypt } from './token'; @@ -163,9 +172,7 @@ export class MockedLicenseBuilder { return this; } - grantedModules: { - module: LicenseModule; - }[] = []; + grantedModules: GrantedModules = []; limits: { activeUsers?: LicenseLimit[]; @@ -190,13 +197,17 @@ export class MockedLicenseBuilder { return this; } - public withGratedModules(modules: LicenseModule[]): this { + public withGratedModules(modules: string[]): this { this.grantedModules = this.grantedModules ?? []; - this.grantedModules.push(...modules.map((module) => ({ module }))); + this.grantedModules.push( + ...(modules.map((module) => + CoreModules.includes(module as LicenseModule) ? { module } : { module, external: true }, + ) as GrantedModules), + ); return this; } - withNoGratedModules(modules: LicenseModule[]): this { + withNoGratedModules(modules: string[]): this { this.grantedModules = this.grantedModules ?? []; this.grantedModules = this.grantedModules.filter(({ module }) => !modules.includes(module)); return this; From 16ea5cc257e475fa18997f1fd290e71686fb840d Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 3 Oct 2024 17:25:43 -0300 Subject: [PATCH 04/25] Improve license types and events for external modules --- ee/packages/license/src/events/emitter.ts | 17 ++++++---- ee/packages/license/src/licenseImp.ts | 8 ++++- ee/packages/license/src/modules.ts | 32 +++++++++++++++++++ .../core-typings/src/license/ILicenseV3.ts | 13 +++----- packages/core-typings/src/license/events.ts | 2 +- 5 files changed, 54 insertions(+), 18 deletions(-) diff --git a/ee/packages/license/src/events/emitter.ts b/ee/packages/license/src/events/emitter.ts index cbf88c0b4677..9090b4502f4e 100644 --- a/ee/packages/license/src/events/emitter.ts +++ b/ee/packages/license/src/events/emitter.ts @@ -1,15 +1,15 @@ -import { CoreModules, type BehaviorWithContext, type LicenseModule } from '@rocket.chat/core-typings'; +import type { BehaviorWithContext } from '@rocket.chat/core-typings'; import type { LicenseManager } from '../license'; import { logger } from '../logger'; - -function isLicenseModule(module: string): module is LicenseModule { - return CoreModules.includes(module as LicenseModule); -} +import { getModuleDefinition, isLicenseModule } from '../modules'; export function moduleValidated(this: LicenseManager, module: string) { + const moduleDefinition = getModuleDefinition.call(this, module); + const external = 'external' in moduleDefinition ? moduleDefinition.external : false; + try { - this.emit('module', { module, valid: true }); + this.emit('module', { module, external, valid: true }); } catch (error) { logger.error({ msg: `Error running module (valid: true) event: ${module}`, error }); } @@ -26,8 +26,11 @@ export function moduleValidated(this: LicenseManager, module: string) { } export function moduleRemoved(this: LicenseManager, module: string) { + const moduleDefinition = getModuleDefinition.call(this, module); + const external = 'external' in moduleDefinition ? moduleDefinition.external : false; + try { - this.emit('module', { module, valid: false }); + this.emit('module', { module, external, valid: false }); } catch (error) { logger.error({ msg: `Error running module (valid: false) event: ${module}`, error }); } diff --git a/ee/packages/license/src/licenseImp.ts b/ee/packages/license/src/licenseImp.ts index f3946c7e1c8e..830566553650 100644 --- a/ee/packages/license/src/licenseImp.ts +++ b/ee/packages/license/src/licenseImp.ts @@ -20,7 +20,7 @@ import { import { overwriteClassOnLicense } from './events/overwriteClassOnLicense'; import { LicenseManager } from './license'; import { logger } from './logger'; -import { getModules, hasModule } from './modules'; +import { getExternalModules, getModuleDefinition, getModules, hasModule } from './modules'; import { showLicense } from './showLicense'; import { getTags } from './tags'; import { getCurrentValueForLicenseLimit, setLicenseLimitCounter } from './validation/getCurrentValueForLicenseLimit'; @@ -31,6 +31,8 @@ interface License { validateFormat: typeof validateFormat; hasModule: typeof hasModule; getModules: typeof getModules; + getModuleDefinition: typeof getModuleDefinition; + getExternalModules: typeof getExternalModules; getTags: typeof getTags; overwriteClassOnLicense: typeof overwriteClassOnLicense; setLicenseLimitCounter: typeof setLicenseLimitCounter; @@ -90,6 +92,10 @@ export class LicenseImp extends LicenseManager implements License { getModules = getModules; + getModuleDefinition = getModuleDefinition; + + getExternalModules = getExternalModules; + getTags = getTags; overwriteClassOnLicense = overwriteClassOnLicense; diff --git a/ee/packages/license/src/modules.ts b/ee/packages/license/src/modules.ts index faa167c1bfc3..45a2846b79d3 100644 --- a/ee/packages/license/src/modules.ts +++ b/ee/packages/license/src/modules.ts @@ -1,6 +1,12 @@ +import { CoreModules, type ExternalModule, type LicenseModule } from '@rocket.chat/core-typings'; + import { moduleRemoved, moduleValidated } from './events/emitter'; import type { LicenseManager } from './license'; +export function isLicenseModule(module: string): module is LicenseModule { + return CoreModules.includes(module as LicenseModule); +} + export function notifyValidatedModules(this: LicenseManager, licenseModules: string[]) { licenseModules.forEach((module) => { this.modules.add(module); @@ -24,6 +30,32 @@ export function getModules(this: LicenseManager) { return [...this.modules]; } +export function getModuleDefinition(this: LicenseManager, moduleName: string) { + const license = this.getLicense(); + + if (!license) { + throw new Error("License not available, can't look for module"); + } + + const moduleDefinition = license.grantedModules.find(({ module }) => module === moduleName); + + if (!moduleDefinition) { + throw new Error('Module not found'); + } + + return moduleDefinition; +} + +export function getExternalModules(this: LicenseManager): ExternalModule[] { + const license = this.getLicense(); + + if (!license) { + return []; + } + + return [...license.grantedModules.filter((value): value is ExternalModule => !isLicenseModule(value.module))]; +} + export function hasModule(this: LicenseManager, module: string) { return this.modules.has(module); } diff --git a/packages/core-typings/src/license/ILicenseV3.ts b/packages/core-typings/src/license/ILicenseV3.ts index 612eb34d2b28..28b786998f21 100644 --- a/packages/core-typings/src/license/ILicenseV3.ts +++ b/packages/core-typings/src/license/ILicenseV3.ts @@ -3,15 +3,10 @@ import type { LicenseLimit } from './LicenseLimit'; import type { LicenseModule } from './LicenseModule'; import type { LicensePeriod, Timestamp } from './LicensePeriod'; -export type GrantedModules = ( - | { - module: LicenseModule; - } - | { - module: string; - external: boolean; - } -)[]; +export type InternalModule = { module: LicenseModule }; +export type ExternalModule = { module: string; external: boolean }; + +export type GrantedModules = (InternalModule | ExternalModule)[]; export interface ILicenseV3 { version: '3.0'; diff --git a/packages/core-typings/src/license/events.ts b/packages/core-typings/src/license/events.ts index 7045262c055a..7d96148b75b5 100644 --- a/packages/core-typings/src/license/events.ts +++ b/packages/core-typings/src/license/events.ts @@ -19,6 +19,6 @@ export type LicenseEvents = ModuleValidation & removed: undefined; validate: undefined; invalidate: undefined; - module: { module: string; valid: boolean }; + module: { module: string; external: boolean; valid: boolean }; sync: undefined; }; From a4e5c6afa95f6d946572ada2b688912363f92981 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 27 Sep 2024 16:14:46 -0300 Subject: [PATCH 05/25] Move appEnableCheck to canEnableApp --- .../ee/app/license/server/canEnableApp.ts | 26 +++++++++--- .../ee/server/apps/communication/rest.ts | 36 ++++------------ .../server/apps/marketplace/appEnableCheck.ts | 42 ------------------- .../src/definition/metadata/IAppInfo.ts | 1 + 4 files changed, 31 insertions(+), 74 deletions(-) delete mode 100644 apps/meteor/ee/server/apps/marketplace/appEnableCheck.ts diff --git a/apps/meteor/ee/app/license/server/canEnableApp.ts b/apps/meteor/ee/app/license/server/canEnableApp.ts index 72220e27acad..c345e271daf6 100644 --- a/apps/meteor/ee/app/license/server/canEnableApp.ts +++ b/apps/meteor/ee/app/license/server/canEnableApp.ts @@ -4,22 +4,38 @@ import { License } from '@rocket.chat/license'; import { getInstallationSourceFromAppStorageItem } from '../../../../lib/apps/getInstallationSourceFromAppStorageItem'; -export const canEnableApp = async (app: IAppStorageItem): Promise => { +export const canEnableApp = async (app: IAppStorageItem): Promise => { if (!(await Apps.isInitialized())) { - return false; + throw new Error('apps-engine-not-initialized'); } // Migrated apps were installed before the validation was implemented // so they're always allowed to be enabled if (app.migrated) { - return true; + return; + } + + if (app.info.addon && !License.hasModule(app.info.addon)) { + throw new Error('app-addon-not-valid'); } const source = getInstallationSourceFromAppStorageItem(app); switch (source) { case 'private': - return !(await License.shouldPreventAction('privateApps')); + if (await License.shouldPreventAction('privateApps')) { + throw new Error('license-prevented'); + } + + break; default: - return !(await License.shouldPreventAction('marketplaceApps')); + if (await License.shouldPreventAction('marketplaceApps')) { + throw new Error('license-prevented'); + } + + if (app.marketplaceInfo?.isEnterpriseOnly && !License.hasValidLicense()) { + throw new Error('invalid-license'); + } + + break; } }; diff --git a/apps/meteor/ee/server/apps/communication/rest.ts b/apps/meteor/ee/server/apps/communication/rest.ts index 3eed45f4ddad..6a60a85c875b 100644 --- a/apps/meteor/ee/server/apps/communication/rest.ts +++ b/apps/meteor/ee/server/apps/communication/rest.ts @@ -1,9 +1,7 @@ import { AppStatus, AppStatusUtils } from '@rocket.chat/apps-engine/definition/AppStatus'; import type { IAppInfo } from '@rocket.chat/apps-engine/definition/metadata'; import type { AppManager } from '@rocket.chat/apps-engine/server/AppManager'; -import { AppInstallationSource } from '@rocket.chat/apps-engine/server/storage'; import type { IUser, IMessage } from '@rocket.chat/core-typings'; -import { License } from '@rocket.chat/license'; import { Settings, Users } from '@rocket.chat/models'; import { serverFetch as fetch } from '@rocket.chat/server-fetch'; import { Meteor } from 'meteor/meteor'; @@ -20,7 +18,6 @@ import { i18n } from '../../../../server/lib/i18n'; import { sendMessagesToAdmins } from '../../../../server/lib/sendMessagesToAdmins'; import { canEnableApp } from '../../../app/license/server/canEnableApp'; import { formatAppInstanceForRest } from '../../../lib/misc/formatAppInstanceForRest'; -import { appEnableCheck } from '../marketplace/appEnableCheck'; import { notifyAppInstall } from '../marketplace/appInstall'; import type { AppServerOrchestrator } from '../orchestrator'; import { Apps } from '../orchestrator'; @@ -418,9 +415,13 @@ export class AppsRestApi { void notifyAppInstall(orchestrator.getMarketplaceUrl() as string, 'install', info); - if (await canEnableApp(aff.getApp().getStorageItem())) { + try { + await canEnableApp(aff.getApp().getStorageItem()); + const success = await manager.enable(info.id); info.status = success ? AppStatus.AUTO_ENABLED : info.status; + } catch (error) { + // should report the error? } void orchestrator.getNotifier().appAdded(info.id); @@ -1157,33 +1158,14 @@ export class AppsRestApi { return API.v1.notFound(`No App found by the id of: ${appId}`); } - const storedApp = prl.getStorageItem(); - const { installationSource, marketplaceInfo } = storedApp; - - if (!License.hasValidLicense() && installationSource === AppInstallationSource.MARKETPLACE) { + if (AppStatusUtils.isEnabled(status)) { try { - const baseUrl = orchestrator.getMarketplaceUrl() as string; - const headers = getDefaultHeaders(); - const { version } = prl.getInfo(); - - await appEnableCheck({ - baseUrl, - headers, - appId, - version, - marketplaceInfo, - status, - logger: orchestrator.getRocketChatLogger(), - }); - } catch (error: any) { - return API.v1.failure(error.message); + await canEnableApp(prl.getStorageItem()); + } catch (error: unknown) { + return API.v1.failure((error as Error).message); } } - if (AppStatusUtils.isEnabled(status) && !(await canEnableApp(storedApp))) { - return API.v1.failure('Enabled apps have been maxed out'); - } - const result = await manager.changeStatus(prl.getID(), status); return API.v1.success({ status: result.getStatus() }); }, diff --git a/apps/meteor/ee/server/apps/marketplace/appEnableCheck.ts b/apps/meteor/ee/server/apps/marketplace/appEnableCheck.ts deleted file mode 100644 index 959b8ff5f8e9..000000000000 --- a/apps/meteor/ee/server/apps/marketplace/appEnableCheck.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; -import type { IMarketplaceInfo } from '@rocket.chat/apps-engine/server/marketplace'; -import type { Logger } from '@rocket.chat/logger'; - -import { getMarketplaceAppInfo } from './appInfo'; - -export const appEnableCheck = async ({ - baseUrl, - headers, - appId, - version, - logger, - status, - marketplaceInfo, -}: { - baseUrl: string; - headers: Record; - appId: string; - version: string; - logger: Logger; - status: AppStatus; - marketplaceInfo?: IMarketplaceInfo; -}) => { - let isAppEnterpriseOnly = false; - - if (marketplaceInfo?.isEnterpriseOnly !== undefined) { - isAppEnterpriseOnly = marketplaceInfo.isEnterpriseOnly; - } else { - try { - const { isEnterpriseOnly } = await getMarketplaceAppInfo({ baseUrl, headers, appId, version }); - - isAppEnterpriseOnly = !!isEnterpriseOnly; - } catch (error: any) { - logger.error('Error getting the app info from the Marketplace:', error.message); - throw new Error(error.message); - } - } - - if (![AppStatus.DISABLED, AppStatus.MANUALLY_DISABLED].includes(status) && isAppEnterpriseOnly) { - throw new Error('Invalid environment for enabling enterprise app'); - } -}; diff --git a/packages/apps-engine/src/definition/metadata/IAppInfo.ts b/packages/apps-engine/src/definition/metadata/IAppInfo.ts index af1d0d62fd11..6cb7515ea6d4 100644 --- a/packages/apps-engine/src/definition/metadata/IAppInfo.ts +++ b/packages/apps-engine/src/definition/metadata/IAppInfo.ts @@ -17,4 +17,5 @@ export interface IAppInfo { iconFileContent?: string; essentials?: Array; permissions?: Array; + addon?: string; } From 22b541f732196c74779307f9cc6bd5687f59bf63 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 3 Oct 2024 20:31:22 -0300 Subject: [PATCH 06/25] When an external module becomes invalid, disable any apps that depend on it --- apps/meteor/ee/server/startup/apps.ts | 28 +++++++++++++++++++ apps/meteor/ee/server/startup/apps/index.ts | 1 - .../ee/server/startup/apps/trialExpiration.ts | 9 ------ ee/packages/license/src/events/listeners.ts | 2 +- 4 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 apps/meteor/ee/server/startup/apps.ts delete mode 100644 apps/meteor/ee/server/startup/apps/index.ts delete mode 100644 apps/meteor/ee/server/startup/apps/trialExpiration.ts diff --git a/apps/meteor/ee/server/startup/apps.ts b/apps/meteor/ee/server/startup/apps.ts new file mode 100644 index 000000000000..634f5c71a4bf --- /dev/null +++ b/apps/meteor/ee/server/startup/apps.ts @@ -0,0 +1,28 @@ +import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; +import { License } from '@rocket.chat/license'; +import { Meteor } from 'meteor/meteor'; + +import { Apps } from '../apps'; + +Meteor.startup(() => { + async function disableAppsCallback() { + void Apps.disableApps(); + } + + License.onInvalidateLicense(disableAppsCallback); + License.onRemoveLicense(disableAppsCallback); + // Disable apps that depend on add-ons (external modules) if they are invalidated + License.onModule(async ({ module, external, valid }) => { + if (!external || valid) return; + + const enabledApps = await Apps.installedApps({ enabled: true }); + + if (!enabledApps) return; + + Promise.all(enabledApps.map(async (app) => { + if (app.getInfo().addon !== module) return; + + await Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); + })); + }); +}); diff --git a/apps/meteor/ee/server/startup/apps/index.ts b/apps/meteor/ee/server/startup/apps/index.ts deleted file mode 100644 index 389658ee535c..000000000000 --- a/apps/meteor/ee/server/startup/apps/index.ts +++ /dev/null @@ -1 +0,0 @@ -import './trialExpiration'; diff --git a/apps/meteor/ee/server/startup/apps/trialExpiration.ts b/apps/meteor/ee/server/startup/apps/trialExpiration.ts deleted file mode 100644 index e6bb9e47c749..000000000000 --- a/apps/meteor/ee/server/startup/apps/trialExpiration.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { License } from '@rocket.chat/license'; -import { Meteor } from 'meteor/meteor'; - -Meteor.startup(() => { - License.onInvalidateLicense(async () => { - const { Apps } = await import('../../apps'); - void Apps.disableApps(); - }); -}); diff --git a/ee/packages/license/src/events/listeners.ts b/ee/packages/license/src/events/listeners.ts index c327789db467..9bfa1ae068b2 100644 --- a/ee/packages/license/src/events/listeners.ts +++ b/ee/packages/license/src/events/listeners.ts @@ -83,7 +83,7 @@ export function onToggledFeature( }; } -export function onModule(this: LicenseManager, cb: (data: { module: string; valid: boolean }) => void) { +export function onModule(this: LicenseManager, cb: (data: { module: string; external: boolean; valid: boolean }) => void) { this.on('module', cb); } From 896ded18823d4b7083b23b854b1a0e1df7b1e0bc Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 09:45:08 -0300 Subject: [PATCH 07/25] Fix getModuleDefinition function --- ee/packages/license/src/events/emitter.ts | 12 ++++++------ ee/packages/license/src/license.spec.ts | 2 ++ ee/packages/license/src/modules.ts | 6 +----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/ee/packages/license/src/events/emitter.ts b/ee/packages/license/src/events/emitter.ts index 9090b4502f4e..31f2b2afc01d 100644 --- a/ee/packages/license/src/events/emitter.ts +++ b/ee/packages/license/src/events/emitter.ts @@ -5,10 +5,10 @@ import { logger } from '../logger'; import { getModuleDefinition, isLicenseModule } from '../modules'; export function moduleValidated(this: LicenseManager, module: string) { - const moduleDefinition = getModuleDefinition.call(this, module); - const external = 'external' in moduleDefinition ? moduleDefinition.external : false; - try { + const moduleDefinition = getModuleDefinition.call(this, module); + const external = moduleDefinition && 'external' in moduleDefinition ? moduleDefinition.external : false; + this.emit('module', { module, external, valid: true }); } catch (error) { logger.error({ msg: `Error running module (valid: true) event: ${module}`, error }); @@ -26,10 +26,10 @@ export function moduleValidated(this: LicenseManager, module: string) { } export function moduleRemoved(this: LicenseManager, module: string) { - const moduleDefinition = getModuleDefinition.call(this, module); - const external = 'external' in moduleDefinition ? moduleDefinition.external : false; - try { + const moduleDefinition = getModuleDefinition.call(this, module); + const external = moduleDefinition && 'external' in moduleDefinition ? moduleDefinition.external : false; + this.emit('module', { module, external, valid: false }); } catch (error) { logger.error({ msg: `Error running module (valid: false) event: ${module}`, error }); diff --git a/ee/packages/license/src/license.spec.ts b/ee/packages/license/src/license.spec.ts index 406dfd696e2e..25fae14b255c 100644 --- a/ee/packages/license/src/license.spec.ts +++ b/ee/packages/license/src/license.spec.ts @@ -444,6 +444,7 @@ describe('License.removeLicense', () => { await expect(moduleCallback).toHaveBeenNthCalledWith(1, { module: 'auditing', valid: true, + external: false, }); removeLicense.mockClear(); @@ -452,6 +453,7 @@ describe('License.removeLicense', () => { await expect(removeLicense).toHaveBeenCalledTimes(1); await expect(moduleCallback).toHaveBeenNthCalledWith(1, { + external: false, module: 'auditing', valid: false, }); diff --git a/ee/packages/license/src/modules.ts b/ee/packages/license/src/modules.ts index 45a2846b79d3..bd834112430b 100644 --- a/ee/packages/license/src/modules.ts +++ b/ee/packages/license/src/modules.ts @@ -34,15 +34,11 @@ export function getModuleDefinition(this: LicenseManager, moduleName: string) { const license = this.getLicense(); if (!license) { - throw new Error("License not available, can't look for module"); + return; } const moduleDefinition = license.grantedModules.find(({ module }) => module === moduleName); - if (!moduleDefinition) { - throw new Error('Module not found'); - } - return moduleDefinition; } From 5637628a9cd35a5028459afb0d07ff5db9fbd6b7 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 10:30:40 -0300 Subject: [PATCH 08/25] Fix code check --- .../NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx | 4 ---- .../header/actions/hooks/useAuditItems.spec.tsx | 4 ---- apps/meteor/ee/server/startup/apps.ts | 10 ++++++---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx b/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx index 94fdfe25a92d..53eb8a6281ab 100644 --- a/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx +++ b/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx @@ -32,7 +32,6 @@ it('should return an empty array of items if have license and not have permissio // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -54,7 +53,6 @@ it('should return auditItems if have license and permissions', async () => { // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -89,7 +87,6 @@ it('should return auditMessages item if have license and can-audit permission', // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -117,7 +114,6 @@ it('should return audiLogs item if have license and can-audit-log permission', a // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) diff --git a/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx b/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx index 66eae7369a3c..fe9bc60fa234 100644 --- a/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx +++ b/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx @@ -32,7 +32,6 @@ it('should return an empty array if have license and not have permissions', asyn // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -54,7 +53,6 @@ it('should return auditItems if have license and permissions', async () => { // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -89,7 +87,6 @@ it('should return auditMessages item if have license and can-audit permission', // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -117,7 +114,6 @@ it('should return audiLogs item if have license and can-audit-log permission', a // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, - // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) diff --git a/apps/meteor/ee/server/startup/apps.ts b/apps/meteor/ee/server/startup/apps.ts index 634f5c71a4bf..ae46c186f80c 100644 --- a/apps/meteor/ee/server/startup/apps.ts +++ b/apps/meteor/ee/server/startup/apps.ts @@ -19,10 +19,12 @@ Meteor.startup(() => { if (!enabledApps) return; - Promise.all(enabledApps.map(async (app) => { - if (app.getInfo().addon !== module) return; + await Promise.all( + enabledApps.map(async (app) => { + if (app.getInfo().addon !== module) return; - await Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); - })); + await Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); + }), + ); }); }); From 0cad25f6218ee46b08662eb1f5ce3d43f6b7817a Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 11:12:34 -0300 Subject: [PATCH 09/25] Add external module to test --- ee/packages/license/src/license.spec.ts | 28 +++++++++++++++++-------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/ee/packages/license/src/license.spec.ts b/ee/packages/license/src/license.spec.ts index 25fae14b255c..7270bbc40246 100644 --- a/ee/packages/license/src/license.spec.ts +++ b/ee/packages/license/src/license.spec.ts @@ -421,7 +421,7 @@ describe('License.setLicense', () => { }); describe('License.removeLicense', () => { - it('should trigger the sync event even if the module callback throws an error', async () => { + it('should trigger the removed event', async () => { const licenseManager = await getReadyLicenseManager(); const removeLicense = jest.fn(); @@ -431,7 +431,7 @@ describe('License.removeLicense', () => { licenseManager.onModule(moduleCallback); - const license = await new MockedLicenseBuilder().withGratedModules(['auditing']).withLimits('activeUsers', [ + const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'chat.rocket.test-addon']).withLimits('activeUsers', [ { max: 10, behavior: 'disable_modules', @@ -440,24 +440,34 @@ describe('License.removeLicense', () => { ]); await expect(licenseManager.setLicense(await license.sign(), true)).resolves.toBe(true); - await expect(removeLicense).toHaveBeenCalledTimes(0); - await expect(moduleCallback).toHaveBeenNthCalledWith(1, { + expect(removeLicense).toHaveBeenCalledTimes(0); + expect(moduleCallback).toHaveBeenNthCalledWith(1, { module: 'auditing', valid: true, external: false, }); + expect(moduleCallback).toHaveBeenNthCalledWith(2, { + module: 'chat.rocket.test-addon', + valid: true, + external: true, + }); removeLicense.mockClear(); moduleCallback.mockClear(); - await licenseManager.remove(); + licenseManager.remove(); - await expect(removeLicense).toHaveBeenCalledTimes(1); - await expect(moduleCallback).toHaveBeenNthCalledWith(1, { - external: false, + expect(removeLicense).toHaveBeenCalledTimes(1); + expect(moduleCallback).toHaveBeenNthCalledWith(1, { module: 'auditing', valid: false, + external: false, + }); + expect(moduleCallback).toHaveBeenNthCalledWith(2, { + module: 'chat.rocket.test-addon', + valid: false, + external: true, }); - await expect(licenseManager.hasValidLicense()).toBe(false); + expect(licenseManager.hasValidLicense()).toBe(false); }); }); From f9a1ded7f9a23c5440865e4933df009cee899ee5 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 11:23:18 -0300 Subject: [PATCH 10/25] Prevent error on startup if app can't be enabled --- apps/meteor/ee/server/apps/orchestrator.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/apps/meteor/ee/server/apps/orchestrator.js b/apps/meteor/ee/server/apps/orchestrator.js index c252579138cb..17744426a34a 100644 --- a/apps/meteor/ee/server/apps/orchestrator.js +++ b/apps/meteor/ee/server/apps/orchestrator.js @@ -177,16 +177,13 @@ export class AppServerOrchestrator { /* eslint-disable no-await-in-loop */ // This needs to happen sequentially to keep track of app limits for (const app of apps) { - const canEnable = await canEnableApp(app.getStorageItem()); + try { + await canEnableApp(app.getStorageItem()); - if (!canEnable) { - this._rocketchatLogger.warn(`App "${app.getInfo().name}" can't be enabled due to CE limits.`); - // We need to continue as the limits are applied depending on the app installation source - // i.e. if one limit is hit, we can't break the loop as the following apps might still be valid - continue; + await this.getManager().loadOne(app.getID()); + } catch (error) { + this._rocketchatLogger.warn(`App "${app.getInfo().name}" could not be enabled: `, error.message); } - - await this.getManager().loadOne(app.getID()); } /* eslint-enable no-await-in-loop */ From b9961803a0bdcdb023472d10ef7d095ccbec4db2 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 11:56:27 -0300 Subject: [PATCH 11/25] Fix external module event trigger --- ee/packages/license/src/events/emitter.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ee/packages/license/src/events/emitter.ts b/ee/packages/license/src/events/emitter.ts index 31f2b2afc01d..ab6329d11641 100644 --- a/ee/packages/license/src/events/emitter.ts +++ b/ee/packages/license/src/events/emitter.ts @@ -2,18 +2,18 @@ import type { BehaviorWithContext } from '@rocket.chat/core-typings'; import type { LicenseManager } from '../license'; import { logger } from '../logger'; -import { getModuleDefinition, isLicenseModule } from '../modules'; +import { isLicenseModule } from '../modules'; export function moduleValidated(this: LicenseManager, module: string) { try { - const moduleDefinition = getModuleDefinition.call(this, module); - const external = moduleDefinition && 'external' in moduleDefinition ? moduleDefinition.external : false; + const external = !isLicenseModule(module); this.emit('module', { module, external, valid: true }); } catch (error) { logger.error({ msg: `Error running module (valid: true) event: ${module}`, error }); } + // Duplicated call for type-guard to work if (!isLicenseModule(module)) { return; } @@ -27,14 +27,14 @@ export function moduleValidated(this: LicenseManager, module: string) { export function moduleRemoved(this: LicenseManager, module: string) { try { - const moduleDefinition = getModuleDefinition.call(this, module); - const external = moduleDefinition && 'external' in moduleDefinition ? moduleDefinition.external : false; + const external = !isLicenseModule(module); this.emit('module', { module, external, valid: false }); } catch (error) { logger.error({ msg: `Error running module (valid: false) event: ${module}`, error }); } + // Duplicated call for type-guard to work if (!isLicenseModule(module)) { return; } From ecc58b23e907e335d75d7b9032b02a07f491f489 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 19:33:36 -0300 Subject: [PATCH 12/25] Increase coverage for PR --- ee/packages/license/src/modules.spec.ts | 76 +++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 ee/packages/license/src/modules.spec.ts diff --git a/ee/packages/license/src/modules.spec.ts b/ee/packages/license/src/modules.spec.ts new file mode 100644 index 000000000000..305fad1b05d0 --- /dev/null +++ b/ee/packages/license/src/modules.spec.ts @@ -0,0 +1,76 @@ +import type { LicenseImp } from '.'; +import { MockedLicenseBuilder, getReadyLicenseManager } from '../__tests__/MockedLicenseBuilder'; + +describe('getModules', () => { + it('should return internal and external', async () => { + const licenseManager = await getReadyLicenseManager(); + + const modules = ['auditing', 'livechat-enterprise', 'ldap-enterprise', 'chat.rocket.test-addon']; + + const license = await new MockedLicenseBuilder().withGratedModules(modules).sign(); + + await expect(licenseManager.setLicense(license)).resolves.toBe(true); + + expect(licenseManager.getModules()).toContain('auditing'); + expect(licenseManager.getModules()).toContain('livechat-enterprise'); + expect(licenseManager.getModules()).toContain('ldap-enterprise'); + expect(licenseManager.getModules()).toContain('chat.rocket.test-addon'); + }); +}); + +describe('getModuleDefinition', () => { + let licenseManager: LicenseImp; + + beforeEach(async () => { + licenseManager = await getReadyLicenseManager(); + + const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'chat.rocket.test-addon']).sign(); + + await licenseManager.setLicense(license); + }); + + it('should not return `external` property for an internal module', () => { + const module = licenseManager.getModuleDefinition('auditing'); + + expect(module).toMatchObject({ module: 'auditing' }); + }); + + it('should return `undefined` for a non-existing module', () => { + const module = licenseManager.getModuleDefinition('livechat-enterprise'); + + expect(module).toBeUndefined(); + }); + + it('should return `external` property for an external module', () => { + const module = licenseManager.getModuleDefinition('chat.rocket.test-addon'); + + expect(module).toMatchObject({ module: 'chat.rocket.test-addon', external: true }); + }); +}); + +describe('getExternalModules', () => { + it('should return only external modules', async () => { + const licenseManager = await getReadyLicenseManager(); + + const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'chat.rocket.test-addon']).sign(); + + await licenseManager.setLicense(license); + + const modules = licenseManager.getExternalModules(); + + expect(modules).toHaveLength(1); + expect(modules[0]).toMatchObject({ external: true, module: 'chat.rocket.test-addon' }); + }); + + it('should return empty array if no external module is present', async () => { + const licenseManager = await getReadyLicenseManager(); + + const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'livechat-enterprise']).sign(); + + await licenseManager.setLicense(license); + + const modules = licenseManager.getExternalModules(); + + expect(modules).toHaveLength(0); + }); +}); From 44cb1661fd6de293f592f3df7009eb02b5521f41 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 19:52:36 -0300 Subject: [PATCH 13/25] Increase MOAR --- ee/packages/license/src/modules.spec.ts | 41 +++++++++++++++++++------ 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/ee/packages/license/src/modules.spec.ts b/ee/packages/license/src/modules.spec.ts index 305fad1b05d0..3863ef19db9e 100644 --- a/ee/packages/license/src/modules.spec.ts +++ b/ee/packages/license/src/modules.spec.ts @@ -1,4 +1,3 @@ -import type { LicenseImp } from '.'; import { MockedLicenseBuilder, getReadyLicenseManager } from '../__tests__/MockedLicenseBuilder'; describe('getModules', () => { @@ -19,29 +18,45 @@ describe('getModules', () => { }); describe('getModuleDefinition', () => { - let licenseManager: LicenseImp; - - beforeEach(async () => { - licenseManager = await getReadyLicenseManager(); + it('should not return `external` property for an internal module', async () => { + const licenseManager = await getReadyLicenseManager(); const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'chat.rocket.test-addon']).sign(); await licenseManager.setLicense(license); - }); - it('should not return `external` property for an internal module', () => { const module = licenseManager.getModuleDefinition('auditing'); expect(module).toMatchObject({ module: 'auditing' }); }); - it('should return `undefined` for a non-existing module', () => { + it('should return `undefined` for a non-existing module', async () => { + const licenseManager = await getReadyLicenseManager(); + + const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'chat.rocket.test-addon']).sign(); + + await licenseManager.setLicense(license); + const module = licenseManager.getModuleDefinition('livechat-enterprise'); expect(module).toBeUndefined(); }); - it('should return `external` property for an external module', () => { + it('should return `undefined` if there is no license available', async () => { + const licenseManager = await getReadyLicenseManager(); + + const module = licenseManager.getModuleDefinition('livechat-enterprise'); + + expect(module).toBeUndefined(); + }); + + it('should return `external` property for an external module', async () => { + const licenseManager = await getReadyLicenseManager(); + + const license = await new MockedLicenseBuilder().withGratedModules(['auditing', 'chat.rocket.test-addon']).sign(); + + await licenseManager.setLicense(license); + const module = licenseManager.getModuleDefinition('chat.rocket.test-addon'); expect(module).toMatchObject({ module: 'chat.rocket.test-addon', external: true }); @@ -73,4 +88,12 @@ describe('getExternalModules', () => { expect(modules).toHaveLength(0); }); + + it('should return empty array if license is not available', async () => { + const licenseManager = await getReadyLicenseManager(); + + const modules = licenseManager.getExternalModules(); + + expect(modules).toHaveLength(0); + }); }); From 1cad980c2d27b435dbccab012e71ede9c52fa4a7 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 4 Oct 2024 20:32:47 -0300 Subject: [PATCH 14/25] Add changeset --- .changeset/three-crews-allow.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/three-crews-allow.md diff --git a/.changeset/three-crews-allow.md b/.changeset/three-crews-allow.md new file mode 100644 index 000000000000..534cac8ec6b6 --- /dev/null +++ b/.changeset/three-crews-allow.md @@ -0,0 +1,8 @@ +--- +'@rocket.chat/core-typings': minor +'@rocket.chat/apps-engine': minor +'@rocket.chat/license': minor +'@rocket.chat/meteor': minor +--- + +Added support for interacting with add-ons issued in the license From c76c2d9d4b51003ca452acbc6b790aea5d593476 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 7 Oct 2024 11:00:55 -0300 Subject: [PATCH 15/25] Return external modules on getInfo --- apps/meteor/tests/mocks/data.ts | 1 + ee/packages/license/src/license.ts | 4 +++- packages/core-typings/src/license/LicenseInfo.ts | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/mocks/data.ts b/apps/meteor/tests/mocks/data.ts index fd6a0ce91123..777fe1967837 100644 --- a/apps/meteor/tests/mocks/data.ts +++ b/apps/meteor/tests/mocks/data.ts @@ -205,6 +205,7 @@ export const createFakeLicenseInfo = (partial: Partial { license: boolean; }): Promise { const activeModules = getModules.call(this); + const externalModules = getExternalModules.call(this); const license = this.getLicense(); // Get all limits present in the license and their current value @@ -495,6 +496,7 @@ export class LicenseManager extends Emitter { return { license: (includeLicense && license) || undefined, activeModules, + externalModules, preventedActions: await this.shouldPreventActionResultsMap(), limits: limits as Record, tags: license?.information.tags || [], diff --git a/packages/core-typings/src/license/LicenseInfo.ts b/packages/core-typings/src/license/LicenseInfo.ts index 85b67dc652eb..6c2cd7ebd943 100644 --- a/packages/core-typings/src/license/LicenseInfo.ts +++ b/packages/core-typings/src/license/LicenseInfo.ts @@ -1,9 +1,10 @@ import type { ILicenseTag } from './ILicenseTag'; -import type { ILicenseV3, LicenseLimitKind } from './ILicenseV3'; +import type { ExternalModule, ILicenseV3, LicenseLimitKind } from './ILicenseV3'; export type LicenseInfo = { license?: ILicenseV3; activeModules: string[]; + externalModules: ExternalModule[]; preventedActions: Record; limits: Record; tags: ILicenseTag[]; From 9396e26511b43fcfc00d61a84eb161339a5d79da Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 7 Oct 2024 15:35:05 -0300 Subject: [PATCH 16/25] Fix loop definition in orchestrator --- apps/meteor/ee/server/apps/orchestrator.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/meteor/ee/server/apps/orchestrator.js b/apps/meteor/ee/server/apps/orchestrator.js index 17744426a34a..62bd6056a986 100644 --- a/apps/meteor/ee/server/apps/orchestrator.js +++ b/apps/meteor/ee/server/apps/orchestrator.js @@ -174,9 +174,8 @@ export class AppServerOrchestrator { // Before enabling each app we verify if there is still room for it const apps = await this.getManager().get(); - /* eslint-disable no-await-in-loop */ // This needs to happen sequentially to keep track of app limits - for (const app of apps) { + for await (const app of apps) { try { await canEnableApp(app.getStorageItem()); @@ -185,7 +184,6 @@ export class AppServerOrchestrator { this._rocketchatLogger.warn(`App "${app.getInfo().name}" could not be enabled: `, error.message); } } - /* eslint-enable no-await-in-loop */ await this.getBridges().getSchedulerBridge().startScheduler(); From 21440a9b4acc85652e4297a5191a83cf2b6a7d08 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 7 Oct 2024 17:03:50 -0300 Subject: [PATCH 17/25] Refactor LicenseModule type --- .../ee/app/license/server/canEnableApp.ts | 3 ++- .../license/src/MockedLicenseBuilder.ts | 9 ++++---- ee/packages/license/src/events/emitter.ts | 22 +++++-------------- ee/packages/license/src/events/listeners.ts | 2 +- ee/packages/license/src/license.ts | 3 ++- ee/packages/license/src/modules.spec.ts | 4 +++- ee/packages/license/src/modules.ts | 19 ++++++++-------- ee/packages/license/src/v2/convertToV3.ts | 4 ++-- .../src/validation/getModulesToDisable.ts | 4 ++-- .../core-typings/src/license/ILicenseV3.ts | 6 ++--- .../src/license/LicenseBehavior.ts | 5 +++-- .../core-typings/src/license/LicenseLimit.ts | 3 ++- .../core-typings/src/license/LicenseModule.ts | 4 +++- .../core-typings/src/license/LicensePeriod.ts | 3 ++- packages/core-typings/src/license/events.ts | 2 +- 15 files changed, 46 insertions(+), 47 deletions(-) diff --git a/apps/meteor/ee/app/license/server/canEnableApp.ts b/apps/meteor/ee/app/license/server/canEnableApp.ts index c345e271daf6..b7922028ef1c 100644 --- a/apps/meteor/ee/app/license/server/canEnableApp.ts +++ b/apps/meteor/ee/app/license/server/canEnableApp.ts @@ -1,5 +1,6 @@ import type { IAppStorageItem } from '@rocket.chat/apps-engine/server/storage'; import { Apps } from '@rocket.chat/core-services'; +import type { LicenseModule } from '@rocket.chat/core-typings'; import { License } from '@rocket.chat/license'; import { getInstallationSourceFromAppStorageItem } from '../../../../lib/apps/getInstallationSourceFromAppStorageItem'; @@ -15,7 +16,7 @@ export const canEnableApp = async (app: IAppStorageItem): Promise => { return; } - if (app.info.addon && !License.hasModule(app.info.addon)) { + if (app.info.addon && !License.hasModule(app.info.addon as LicenseModule)) { throw new Error('app-addon-not-valid'); } diff --git a/ee/packages/license/src/MockedLicenseBuilder.ts b/ee/packages/license/src/MockedLicenseBuilder.ts index 1b83900993aa..d9def5b6b0d5 100644 --- a/ee/packages/license/src/MockedLicenseBuilder.ts +++ b/ee/packages/license/src/MockedLicenseBuilder.ts @@ -1,3 +1,4 @@ +import type { InternalModuleName } from '@rocket.chat/core-typings'; import { CoreModules, type GrantedModules, @@ -197,17 +198,15 @@ export class MockedLicenseBuilder { return this; } - public withGratedModules(modules: string[]): this { + public withGratedModules(modules: LicenseModule[]): this { this.grantedModules = this.grantedModules ?? []; this.grantedModules.push( - ...(modules.map((module) => - CoreModules.includes(module as LicenseModule) ? { module } : { module, external: true }, - ) as GrantedModules), + ...(modules.map((module) => ({ module, external: !CoreModules.includes(module as InternalModuleName) })) as GrantedModules), ); return this; } - withNoGratedModules(modules: string[]): this { + withNoGratedModules(modules: LicenseModule[]): this { this.grantedModules = this.grantedModules ?? []; this.grantedModules = this.grantedModules.filter(({ module }) => !modules.includes(module)); return this; diff --git a/ee/packages/license/src/events/emitter.ts b/ee/packages/license/src/events/emitter.ts index ab6329d11641..fee830618eb8 100644 --- a/ee/packages/license/src/events/emitter.ts +++ b/ee/packages/license/src/events/emitter.ts @@ -1,23 +1,18 @@ -import type { BehaviorWithContext } from '@rocket.chat/core-typings'; +import type { BehaviorWithContext, LicenseModule } from '@rocket.chat/core-typings'; import type { LicenseManager } from '../license'; import { logger } from '../logger'; -import { isLicenseModule } from '../modules'; +import { isInternalModuleName } from '../modules'; -export function moduleValidated(this: LicenseManager, module: string) { +export function moduleValidated(this: LicenseManager, module: LicenseModule) { try { - const external = !isLicenseModule(module); + const external = !isInternalModuleName(module); this.emit('module', { module, external, valid: true }); } catch (error) { logger.error({ msg: `Error running module (valid: true) event: ${module}`, error }); } - // Duplicated call for type-guard to work - if (!isLicenseModule(module)) { - return; - } - try { this.emit(`valid:${module}`); } catch (error) { @@ -25,20 +20,15 @@ export function moduleValidated(this: LicenseManager, module: string) { } } -export function moduleRemoved(this: LicenseManager, module: string) { +export function moduleRemoved(this: LicenseManager, module: LicenseModule) { try { - const external = !isLicenseModule(module); + const external = !isInternalModuleName(module); this.emit('module', { module, external, valid: false }); } catch (error) { logger.error({ msg: `Error running module (valid: false) event: ${module}`, error }); } - // Duplicated call for type-guard to work - if (!isLicenseModule(module)) { - return; - } - try { this.emit(`invalid:${module}`); } catch (error) { diff --git a/ee/packages/license/src/events/listeners.ts b/ee/packages/license/src/events/listeners.ts index 9bfa1ae068b2..97e57afbcc6d 100644 --- a/ee/packages/license/src/events/listeners.ts +++ b/ee/packages/license/src/events/listeners.ts @@ -83,7 +83,7 @@ export function onToggledFeature( }; } -export function onModule(this: LicenseManager, cb: (data: { module: string; external: boolean; valid: boolean }) => void) { +export function onModule(this: LicenseManager, cb: (data: { module: LicenseModule; external: boolean; valid: boolean }) => void) { this.on('module', cb); } diff --git a/ee/packages/license/src/license.ts b/ee/packages/license/src/license.ts index 1a234ec5ffb6..beb13a63e83d 100644 --- a/ee/packages/license/src/license.ts +++ b/ee/packages/license/src/license.ts @@ -9,6 +9,7 @@ import type { LicenseInfo, LicenseValidationOptions, LimitContext, + LicenseModule, } from '@rocket.chat/core-typings'; import { Emitter } from '@rocket.chat/emitter'; @@ -42,7 +43,7 @@ export class LicenseManager extends Emitter { tags = new Set(); - modules = new Set(); + modules = new Set(); private workspaceUrl: string | undefined; diff --git a/ee/packages/license/src/modules.spec.ts b/ee/packages/license/src/modules.spec.ts index 3863ef19db9e..07e1fe6d9170 100644 --- a/ee/packages/license/src/modules.spec.ts +++ b/ee/packages/license/src/modules.spec.ts @@ -1,10 +1,12 @@ +import type { LicenseModule } from '@rocket.chat/core-typings'; + import { MockedLicenseBuilder, getReadyLicenseManager } from '../__tests__/MockedLicenseBuilder'; describe('getModules', () => { it('should return internal and external', async () => { const licenseManager = await getReadyLicenseManager(); - const modules = ['auditing', 'livechat-enterprise', 'ldap-enterprise', 'chat.rocket.test-addon']; + const modules = ['auditing', 'livechat-enterprise', 'ldap-enterprise', 'chat.rocket.test-addon'] as LicenseModule[]; const license = await new MockedLicenseBuilder().withGratedModules(modules).sign(); diff --git a/ee/packages/license/src/modules.ts b/ee/packages/license/src/modules.ts index bd834112430b..15cebdcde548 100644 --- a/ee/packages/license/src/modules.ts +++ b/ee/packages/license/src/modules.ts @@ -1,20 +1,21 @@ -import { CoreModules, type ExternalModule, type LicenseModule } from '@rocket.chat/core-typings'; +import type { LicenseModule, InternalModuleName, ExternalModule } from '@rocket.chat/core-typings'; +import { CoreModules } from '@rocket.chat/core-typings'; import { moduleRemoved, moduleValidated } from './events/emitter'; import type { LicenseManager } from './license'; -export function isLicenseModule(module: string): module is LicenseModule { - return CoreModules.includes(module as LicenseModule); +export function isInternalModuleName(module: string): module is InternalModuleName { + return CoreModules.includes(module as InternalModuleName); } -export function notifyValidatedModules(this: LicenseManager, licenseModules: string[]) { +export function notifyValidatedModules(this: LicenseManager, licenseModules: LicenseModule[]) { licenseModules.forEach((module) => { this.modules.add(module); moduleValidated.call(this, module); }); } -export function notifyInvalidatedModules(this: LicenseManager, licenseModules: string[]) { +export function notifyInvalidatedModules(this: LicenseManager, licenseModules: LicenseModule[]) { licenseModules.forEach((module) => { moduleRemoved.call(this, module); this.modules.delete(module); @@ -30,7 +31,7 @@ export function getModules(this: LicenseManager) { return [...this.modules]; } -export function getModuleDefinition(this: LicenseManager, moduleName: string) { +export function getModuleDefinition(this: LicenseManager, moduleName: LicenseModule) { const license = this.getLicense(); if (!license) { @@ -49,14 +50,14 @@ export function getExternalModules(this: LicenseManager): ExternalModule[] { return []; } - return [...license.grantedModules.filter((value): value is ExternalModule => !isLicenseModule(value.module))]; + return [...license.grantedModules.filter((value): value is ExternalModule => !isInternalModuleName(value.module))]; } -export function hasModule(this: LicenseManager, module: string) { +export function hasModule(this: LicenseManager, module: LicenseModule) { return this.modules.has(module); } -export function replaceModules(this: LicenseManager, newModules: string[]): boolean { +export function replaceModules(this: LicenseManager, newModules: LicenseModule[]): boolean { let anyChange = false; for (const moduleName of newModules) { if (this.modules.has(moduleName)) { diff --git a/ee/packages/license/src/v2/convertToV3.ts b/ee/packages/license/src/v2/convertToV3.ts index a7b67ada2c6e..eccde563363c 100644 --- a/ee/packages/license/src/v2/convertToV3.ts +++ b/ee/packages/license/src/v2/convertToV3.ts @@ -3,7 +3,7 @@ * Transform a License V2 into a V3 representation. */ -import type { ILicenseV2, ILicenseV3, LicenseModule } from '@rocket.chat/core-typings'; +import type { ILicenseV2, ILicenseV3, InternalModuleName } from '@rocket.chat/core-typings'; import { isBundle, getBundleFromModule, getBundleModules } from './bundles'; import { getTagColor } from './getTagColor'; @@ -53,7 +53,7 @@ export const convertToV3 = (v2: ILicenseV2): ILicenseV3 => { ['hide-watermark', ...v2.modules] .map((licenseModule) => (isBundle(licenseModule) ? getBundleModules(licenseModule) : [licenseModule])) .reduce((prev, curr) => [...prev, ...curr], []) - .map((licenseModule) => ({ module: licenseModule as LicenseModule })), + .map((licenseModule) => ({ module: licenseModule as InternalModuleName })), ), ], limits: { diff --git a/ee/packages/license/src/validation/getModulesToDisable.ts b/ee/packages/license/src/validation/getModulesToDisable.ts index 7ce8f98abe79..cdab814db026 100644 --- a/ee/packages/license/src/validation/getModulesToDisable.ts +++ b/ee/packages/license/src/validation/getModulesToDisable.ts @@ -1,9 +1,9 @@ -import type { BehaviorWithContext, LicenseBehavior } from '@rocket.chat/core-typings'; +import type { BehaviorWithContext, LicenseBehavior, LicenseModule } from '@rocket.chat/core-typings'; const filterValidationResult = (result: BehaviorWithContext[], expectedBehavior: LicenseBehavior) => result.filter(({ behavior }) => behavior === expectedBehavior) as BehaviorWithContext[]; -export const getModulesToDisable = (validationResult: BehaviorWithContext[]): string[] => { +export const getModulesToDisable = (validationResult: BehaviorWithContext[]): LicenseModule[] => { return [ ...new Set([ ...filterValidationResult(validationResult, 'disable_modules') diff --git a/packages/core-typings/src/license/ILicenseV3.ts b/packages/core-typings/src/license/ILicenseV3.ts index 28b786998f21..b6f6fae00eb7 100644 --- a/packages/core-typings/src/license/ILicenseV3.ts +++ b/packages/core-typings/src/license/ILicenseV3.ts @@ -1,10 +1,10 @@ import type { ILicenseTag } from './ILicenseTag'; import type { LicenseLimit } from './LicenseLimit'; -import type { LicenseModule } from './LicenseModule'; +import type { ExternalModuleName, InternalModuleName } from './LicenseModule'; import type { LicensePeriod, Timestamp } from './LicensePeriod'; -export type InternalModule = { module: LicenseModule }; -export type ExternalModule = { module: string; external: boolean }; +export type InternalModule = { module: InternalModuleName; external?: false }; +export type ExternalModule = { module: ExternalModuleName; external: true }; export type GrantedModules = (InternalModule | ExternalModule)[]; diff --git a/packages/core-typings/src/license/LicenseBehavior.ts b/packages/core-typings/src/license/LicenseBehavior.ts index 915f4e116073..ac2249233ab5 100644 --- a/packages/core-typings/src/license/LicenseBehavior.ts +++ b/packages/core-typings/src/license/LicenseBehavior.ts @@ -1,4 +1,5 @@ import type { LicenseLimitKind } from './ILicenseV3'; +import type { LicenseModule } from './LicenseModule'; export type LicenseBehavior = | 'invalidate_license' @@ -11,12 +12,12 @@ export type LicenseBehavior = export type BehaviorWithContext = | { behavior: LicenseBehavior; - modules?: string[]; + modules?: LicenseModule[]; reason: 'limit'; limit?: LicenseLimitKind; } | { behavior: LicenseBehavior; - modules?: string[]; + modules?: LicenseModule[]; reason: 'period' | 'url'; }; diff --git a/packages/core-typings/src/license/LicenseLimit.ts b/packages/core-typings/src/license/LicenseLimit.ts index 76ff615f51b4..40e5a62f597a 100644 --- a/packages/core-typings/src/license/LicenseLimit.ts +++ b/packages/core-typings/src/license/LicenseLimit.ts @@ -1,6 +1,7 @@ import type { LicenseBehavior } from './LicenseBehavior'; +import type { LicenseModule } from './LicenseModule'; export type LicenseLimit = { max: number; behavior: T; -} & (T extends 'disable_modules' ? { behavior: T; modules: string[] } : { behavior: T }); +} & (T extends 'disable_modules' ? { behavior: T; modules: LicenseModule[] } : { behavior: T }); diff --git a/packages/core-typings/src/license/LicenseModule.ts b/packages/core-typings/src/license/LicenseModule.ts index c5e7495d2b2d..d5d8da893d71 100644 --- a/packages/core-typings/src/license/LicenseModule.ts +++ b/packages/core-typings/src/license/LicenseModule.ts @@ -22,4 +22,6 @@ export const CoreModules = [ 'unlimited-presence', ] as const; -export type LicenseModule = (typeof CoreModules)[number]; +export type InternalModuleName = (typeof CoreModules)[number]; +export type ExternalModuleName = `${string}.${string}`; +export type LicenseModule = InternalModuleName | ExternalModuleName; diff --git a/packages/core-typings/src/license/LicensePeriod.ts b/packages/core-typings/src/license/LicensePeriod.ts index b7258e4ef6bf..d9bae6198fde 100644 --- a/packages/core-typings/src/license/LicensePeriod.ts +++ b/packages/core-typings/src/license/LicensePeriod.ts @@ -1,4 +1,5 @@ import type { LicenseBehavior } from './LicenseBehavior'; +import type { LicenseModule } from './LicenseModule'; export type Timestamp = string; @@ -7,6 +8,6 @@ export type LicensePeriod = { validUntil?: Timestamp; invalidBehavior: LicenseBehavior; } & ({ validFrom: Timestamp } | { validUntil: Timestamp }) & - ({ invalidBehavior: 'disable_modules'; modules: string[] } | { invalidBehavior: Exclude }); + ({ invalidBehavior: 'disable_modules'; modules: LicenseModule[] } | { invalidBehavior: Exclude }); export type LicensePeriodBehavior = Exclude; diff --git a/packages/core-typings/src/license/events.ts b/packages/core-typings/src/license/events.ts index 7d96148b75b5..487d31ae6698 100644 --- a/packages/core-typings/src/license/events.ts +++ b/packages/core-typings/src/license/events.ts @@ -19,6 +19,6 @@ export type LicenseEvents = ModuleValidation & removed: undefined; validate: undefined; invalidate: undefined; - module: { module: string; external: boolean; valid: boolean }; + module: { module: LicenseModule; external: boolean; valid: boolean }; sync: undefined; }; From e6d49434cd7cb034414387ae9cb8056aa04bac5f Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Tue, 8 Oct 2024 20:10:34 -0300 Subject: [PATCH 18/25] Refactor type in LicenseInfo --- .../NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx | 4 ++++ .../sidebar/header/actions/hooks/useAuditItems.spec.tsx | 4 ++++ packages/core-typings/src/license/LicenseInfo.ts | 3 ++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx b/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx index 53eb8a6281ab..94fdfe25a92d 100644 --- a/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx +++ b/apps/meteor/client/NavBarV2/NavBarPagesToolbar/hooks/useAuditMenu.spec.tsx @@ -32,6 +32,7 @@ it('should return an empty array of items if have license and not have permissio // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -53,6 +54,7 @@ it('should return auditItems if have license and permissions', async () => { // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -87,6 +89,7 @@ it('should return auditMessages item if have license and can-audit permission', // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -114,6 +117,7 @@ it('should return audiLogs item if have license and can-audit-log permission', a // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) diff --git a/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx b/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx index fe9bc60fa234..66eae7369a3c 100644 --- a/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx +++ b/apps/meteor/client/sidebar/header/actions/hooks/useAuditItems.spec.tsx @@ -32,6 +32,7 @@ it('should return an empty array if have license and not have permissions', asyn // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -53,6 +54,7 @@ it('should return auditItems if have license and permissions', async () => { // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -87,6 +89,7 @@ it('should return auditMessages item if have license and can-audit permission', // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) @@ -114,6 +117,7 @@ it('should return audiLogs item if have license and can-audit-log permission', a // @ts-expect-error: just for testing grantedModules: [{ module: 'auditing' }], }, + // @ts-expect-error: just for testing activeModules: ['auditing'], }, })) diff --git a/packages/core-typings/src/license/LicenseInfo.ts b/packages/core-typings/src/license/LicenseInfo.ts index 6c2cd7ebd943..85e7c3fba506 100644 --- a/packages/core-typings/src/license/LicenseInfo.ts +++ b/packages/core-typings/src/license/LicenseInfo.ts @@ -1,9 +1,10 @@ import type { ILicenseTag } from './ILicenseTag'; import type { ExternalModule, ILicenseV3, LicenseLimitKind } from './ILicenseV3'; +import type { LicenseModule } from './LicenseModule'; export type LicenseInfo = { license?: ILicenseV3; - activeModules: string[]; + activeModules: LicenseModule[]; externalModules: ExternalModule[]; preventedActions: Record; limits: Record; From 99b8e32c94f4ac273ccc6846a800840ec5444215 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 9 Oct 2024 17:47:33 -0300 Subject: [PATCH 19/25] Add unit tests for canEnableApp --- .../ee/app/license/server/canEnableApp.ts | 11 +- .../app/license/server/canEnableApp.spec.ts | 129 ++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 apps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts diff --git a/apps/meteor/ee/app/license/server/canEnableApp.ts b/apps/meteor/ee/app/license/server/canEnableApp.ts index b7922028ef1c..c4ad4d5bcf79 100644 --- a/apps/meteor/ee/app/license/server/canEnableApp.ts +++ b/apps/meteor/ee/app/license/server/canEnableApp.ts @@ -1,11 +1,16 @@ import type { IAppStorageItem } from '@rocket.chat/apps-engine/server/storage'; import { Apps } from '@rocket.chat/core-services'; import type { LicenseModule } from '@rocket.chat/core-typings'; -import { License } from '@rocket.chat/license'; +import { License, type LicenseImp } from '@rocket.chat/license'; import { getInstallationSourceFromAppStorageItem } from '../../../../lib/apps/getInstallationSourceFromAppStorageItem'; -export const canEnableApp = async (app: IAppStorageItem): Promise => { +type _canEnableAppDependencies = { + Apps: typeof Apps; + License: LicenseImp; +}; + +export const _canEnableApp = async ({ Apps, License }: _canEnableAppDependencies, app: IAppStorageItem): Promise => { if (!(await Apps.isInitialized())) { throw new Error('apps-engine-not-initialized'); } @@ -40,3 +45,5 @@ export const canEnableApp = async (app: IAppStorageItem): Promise => { break; } }; + +export const canEnableApp = async (app: IAppStorageItem): Promise => _canEnableApp({ Apps, License }, app); diff --git a/apps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts b/apps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts new file mode 100644 index 000000000000..8e5b2c0bc3ab --- /dev/null +++ b/apps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts @@ -0,0 +1,129 @@ +import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; +import type { IMarketplaceInfo } from '@rocket.chat/apps-engine/server/marketplace'; +import { AppInstallationSource, type IAppStorageItem } from '@rocket.chat/apps-engine/server/storage'; +import type { Apps } from '@rocket.chat/core-services'; +import type { LicenseImp } from '@rocket.chat/license'; +import { expect } from 'chai'; + +import { _canEnableApp } from '../../../../../ee/app/license/server/canEnableApp'; + +const getDefaultApp = (): IAppStorageItem => ({ + _id: '6706d9258e0ca97c2f0cc885', + id: '2e14ff6e-b4d5-4c4c-b12b-b1b1d15ec630', + info: { + id: '2e14ff6e-b4d5-4c4c-b12b-b1b1d15ec630', + version: '0.0.1', + requiredApiVersion: '^1.19.0', + iconFile: 'icon.png', + author: { name: 'a', homepage: 'a', support: 'a' }, + name: 'Add-on test', + nameSlug: 'add-on-test', + classFile: 'AddOnTestApp.js', + description: 'a', + implements: [], + iconFileContent: '', + }, + status: AppStatus.UNKNOWN, + settings: {}, + implemented: {}, + installationSource: AppInstallationSource.PRIVATE, + languageContent: {}, + sourcePath: 'GridFS:/6706d9258e0ca97c2f0cc880', + signature: '', + createdAt: new Date('2024-10-09T19:27:33.923Z'), + updatedAt: new Date('2024-10-09T19:27:33.923Z'), +}); + +// We will be passing promises to the `expect` function +/* eslint-disable @typescript-eslint/no-floating-promises */ + +describe('canEnableApp', () => { + it('should throw the message "apps-engine-not-initialized" when appropriate', () => { + const AppsMock = { + isInitialized() { + return false; + }, + } as unknown as typeof Apps; + + const LicenseMock = {} as unknown as LicenseImp; + + const deps = { Apps: AppsMock, License: LicenseMock }; + + return expect(_canEnableApp(deps, getDefaultApp())).to.eventually.be.rejectedWith('apps-engine-not-initialized'); + }); + + const AppsMock = { + isInitialized() { + return true; + }, + } as unknown as typeof Apps; + + const LicenseMock = { + hasModule() { + return false; + }, + shouldPreventAction() { + return true; + }, + hasValidLicense() { + return false; + }, + } as unknown as LicenseImp; + + const deps = { Apps: AppsMock, License: LicenseMock }; + + it('should throw the message "app-addon-not-valid" when appropriate', () => { + const app = getDefaultApp(); + app.info.addon = 'chat.rocket.test-addon'; + + return expect(_canEnableApp(deps, app)).to.eventually.be.rejectedWith('app-addon-not-valid'); + }); + + it('should throw the message "license-prevented" when appropriate', () => { + const privateApp = getDefaultApp(); + const marketplaceApp = getDefaultApp(); + marketplaceApp.installationSource = AppInstallationSource.MARKETPLACE; + + return Promise.all([ + expect(_canEnableApp(deps, privateApp)).to.eventually.be.rejectedWith('license-prevented'), + expect(_canEnableApp(deps, marketplaceApp)).to.eventually.be.rejectedWith('license-prevented'), + ]); + }); + + it('should throw the message "invalid-license" when appropriate', () => { + const License = { ...LicenseMock, shouldPreventAction: () => false } as unknown as LicenseImp; + + const app = getDefaultApp(); + app.installationSource = AppInstallationSource.MARKETPLACE; + app.marketplaceInfo = { isEnterpriseOnly: true } as IMarketplaceInfo; + + const deps = { Apps: AppsMock, License }; + + return expect(_canEnableApp(deps, app)).to.eventually.be.rejectedWith('invalid-license'); + }); + + it('should not throw if app is migrated', () => { + const app = getDefaultApp(); + app.migrated = true; + + return expect(_canEnableApp(deps, app)).to.not.eventually.be.rejected; + }); + + it('should not throw if license allows it', () => { + const License = { + hasModule() { + return true; + }, + shouldPreventAction() { + return false; + }, + hasValidLicense() { + return true; + }, + } as unknown as LicenseImp; + + const deps = { Apps: AppsMock, License }; + + return expect(_canEnableApp(deps, getDefaultApp())).to.not.eventually.be.rejected; + }); +}); From 3c5a30e0b4bd36e62dcf9d74c1fb74924d255a25 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 10 Oct 2024 12:12:34 -0300 Subject: [PATCH 20/25] Add unit tests for apps license module change callback --- .../apps/makeDisableAppsWithAddonsCallback.ts | 23 ++++ apps/meteor/ee/server/startup/apps.ts | 18 +-- .../disableAppsWithAddonsCallback.spec.ts | 119 ++++++++++++++++++ 3 files changed, 144 insertions(+), 16 deletions(-) create mode 100644 apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts create mode 100644 apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts diff --git a/apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts b/apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts new file mode 100644 index 000000000000..8009f8344eee --- /dev/null +++ b/apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts @@ -0,0 +1,23 @@ +import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; +import type { LicenseImp } from '@rocket.chat/license'; + +import type { Apps } from '../../apps'; + +type OnModuleCallbackParameter = Parameters[0]>[0]; + +export const makeDisableAppsWithAddonsCallback = (deps: { Apps: typeof Apps }) => + async function disableAppsWithAddonsCallback({ module, external, valid }: OnModuleCallbackParameter) { + if (!external || valid) return; + + const enabledApps = await deps.Apps.installedApps({ enabled: true }); + + if (!enabledApps) return; + + await Promise.all( + enabledApps.map(async (app) => { + if (app.getInfo().addon !== module) return; + + return deps.Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); + }), + ); + }; diff --git a/apps/meteor/ee/server/startup/apps.ts b/apps/meteor/ee/server/startup/apps.ts index ae46c186f80c..82c6ecf2b4c2 100644 --- a/apps/meteor/ee/server/startup/apps.ts +++ b/apps/meteor/ee/server/startup/apps.ts @@ -1,8 +1,8 @@ -import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; import { License } from '@rocket.chat/license'; import { Meteor } from 'meteor/meteor'; import { Apps } from '../apps'; +import { makeDisableAppsWithAddonsCallback } from '../lib/apps/makeDisableAppsWithAddonsCallback'; Meteor.startup(() => { async function disableAppsCallback() { @@ -12,19 +12,5 @@ Meteor.startup(() => { License.onInvalidateLicense(disableAppsCallback); License.onRemoveLicense(disableAppsCallback); // Disable apps that depend on add-ons (external modules) if they are invalidated - License.onModule(async ({ module, external, valid }) => { - if (!external || valid) return; - - const enabledApps = await Apps.installedApps({ enabled: true }); - - if (!enabledApps) return; - - await Promise.all( - enabledApps.map(async (app) => { - if (app.getInfo().addon !== module) return; - - await Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); - }), - ); - }); + License.onModule(makeDisableAppsWithAddonsCallback({ Apps })); }); diff --git a/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts b/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts new file mode 100644 index 000000000000..20149f722ff1 --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts @@ -0,0 +1,119 @@ +import { expect, spy } from 'chai'; + +import { makeDisableAppsWithAddonsCallback } from '../../../../../../ee/server/lib/apps/makeDisableAppsWithAddonsCallback'; +import type { AppServerOrchestrator } from '../../../../../../ee/server/apps/orchestrator'; + +describe('disableAppsWithAddonsCallback', () => { + it('should not execute anything if not external module', async () => { + function installedApps() { + return []; + } + + function getManagerDisable() { + return undefined; + } + + const AppsMock = { + installedApps: spy(installedApps), + getManager: () => ({ + disable: spy(getManagerDisable), + }), + } as unknown as AppServerOrchestrator; + + const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); + + await disableAppsWithAddonsCallback({ module: 'auditing', external: false, valid: true }); + + expect(AppsMock.installedApps).to.not.have.been.called(); + expect(AppsMock.getManager()!.disable).to.not.have.been.called(); + }); + + it('should not execute anything if module is external and valid', async () => { + function installedApps() { + return []; + } + + function getManagerDisable() { + return undefined; + } + + const AppsMock = { + installedApps: spy(installedApps), + getManager: () => ({ + disable: spy(getManagerDisable), + }), + } as unknown as AppServerOrchestrator; + + const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); + + await disableAppsWithAddonsCallback({ module: 'auditing', external: true, valid: true }); + + expect(AppsMock.installedApps).to.not.have.been.called(); + expect(AppsMock.getManager()!.disable).to.not.have.been.called(); + }); + + it('should not throw if there are no apps installed that are enabled', async () => { + function installedApps() { + return []; + } + + function getManagerDisable() { + return undefined; + } + + const AppsMock = { + installedApps: spy(installedApps), + getManager: () => ({ + disable: spy(getManagerDisable), + }), + } as unknown as AppServerOrchestrator; + + const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); + + await expect(disableAppsWithAddonsCallback({ module: 'auditing', external: true, valid: false })).to.not.eventually.be.rejected; + + expect(AppsMock.installedApps).to.have.been.called(); + expect(AppsMock.getManager()!.disable).to.not.have.been.called(); + }); + + it('should only disable apps that require addons', async () => { + function installedApps() { + return [ + { + getInfo: () => ({}), + getID() { + return 'test-app-without-addon'; + }, + }, + { + getInfo: () => ({ addon: 'chat.rocket.test-addon' }), + getID() { + return 'test-app-with-addon'; + }, + }, + ]; + } + + function getManagerDisable() { + return undefined; + } + + const mockManager = { + disable: spy(getManagerDisable), + }; + + const AppsMock = { + installedApps: spy(installedApps), + getManager: () => mockManager, + } as unknown as AppServerOrchestrator; + + const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); + + await expect(disableAppsWithAddonsCallback({ module: 'chat.rocket.test-addon', external: true, valid: false })).to.not.eventually.be + .rejected; + + expect(AppsMock.installedApps).to.have.been.called(); + expect(AppsMock.getManager()!.disable).to.have.been.called.once; + expect(AppsMock.getManager()!.disable).to.have.been.called.with('test-app-with-addon'); + }); +}); From 459716eedf4af61f50f935d6616d895370f3b17c Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 10 Oct 2024 12:22:16 -0300 Subject: [PATCH 21/25] Log to the server if the app is installed but can't be enabled --- apps/meteor/ee/server/apps/communication/rest.ts | 2 +- .../app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/ee/server/apps/communication/rest.ts b/apps/meteor/ee/server/apps/communication/rest.ts index 6a60a85c875b..d07910cfcd0e 100644 --- a/apps/meteor/ee/server/apps/communication/rest.ts +++ b/apps/meteor/ee/server/apps/communication/rest.ts @@ -421,7 +421,7 @@ export class AppsRestApi { const success = await manager.enable(info.id); info.status = success ? AppStatus.AUTO_ENABLED : info.status; } catch (error) { - // should report the error? + orchestrator.getRocketChatLogger().warn(`App "${info.id}" was installed but could not be enabled: `, error); } void orchestrator.getNotifier().appAdded(info.id); diff --git a/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts b/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts index 20149f722ff1..5dbde66da537 100644 --- a/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts @@ -1,7 +1,7 @@ import { expect, spy } from 'chai'; -import { makeDisableAppsWithAddonsCallback } from '../../../../../../ee/server/lib/apps/makeDisableAppsWithAddonsCallback'; import type { AppServerOrchestrator } from '../../../../../../ee/server/apps/orchestrator'; +import { makeDisableAppsWithAddonsCallback } from '../../../../../../ee/server/lib/apps/makeDisableAppsWithAddonsCallback'; describe('disableAppsWithAddonsCallback', () => { it('should not execute anything if not external module', async () => { From de510c2b439f29b37e3670ccae640e069f744d7f Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 17 Oct 2024 12:50:18 -0300 Subject: [PATCH 22/25] feat: send message to admins when app is disabled due to add-on (#33534) --- .../lib/apps/disableAppsWithAddonsCallback.ts | 46 +++++ .../apps/makeDisableAppsWithAddonsCallback.ts | 23 --- apps/meteor/ee/server/startup/apps.ts | 4 +- .../disableAppsWithAddonsCallback.spec.ts | 168 +++++++++++++++--- packages/i18n/src/locales/en.i18n.json | 2 + 5 files changed, 191 insertions(+), 52 deletions(-) create mode 100644 apps/meteor/ee/server/lib/apps/disableAppsWithAddonsCallback.ts delete mode 100644 apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts diff --git a/apps/meteor/ee/server/lib/apps/disableAppsWithAddonsCallback.ts b/apps/meteor/ee/server/lib/apps/disableAppsWithAddonsCallback.ts new file mode 100644 index 000000000000..03f7011ba727 --- /dev/null +++ b/apps/meteor/ee/server/lib/apps/disableAppsWithAddonsCallback.ts @@ -0,0 +1,46 @@ +import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; +import type { LicenseImp } from '@rocket.chat/license'; + +import { i18n } from '../../../../server/lib/i18n'; +import { sendMessagesToAdmins } from '../../../../server/lib/sendMessagesToAdmins'; +import { Apps } from '../../apps'; + +type OnModuleCallbackParameter = Parameters[0]>[0]; + +export async function _disableAppsWithAddonsCallback( + deps: { Apps: typeof Apps; sendMessagesToAdmins: typeof sendMessagesToAdmins }, + { module, external, valid }: OnModuleCallbackParameter, +) { + if (!external || valid) return; + + const enabledApps = await deps.Apps.installedApps({ enabled: true }); + + if (!enabledApps) return; + + const affectedApps: string[] = []; + + await Promise.all( + enabledApps.map(async (app) => { + if (app.getInfo().addon !== module) return; + + affectedApps.push(app.getName()); + + return deps.Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); + }), + ); + + if (!affectedApps.length) return; + + await deps.sendMessagesToAdmins({ + msgs: async ({ adminUser }) => ({ + msg: i18n.t('App_has_been_disabled_addon_message', { + lng: adminUser.language || 'en', + count: affectedApps.length, + appNames: affectedApps, + }), + }), + }); +} + +export const disableAppsWithAddonsCallback = (ctx: OnModuleCallbackParameter) => + _disableAppsWithAddonsCallback({ Apps, sendMessagesToAdmins }, ctx); diff --git a/apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts b/apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts deleted file mode 100644 index 8009f8344eee..000000000000 --- a/apps/meteor/ee/server/lib/apps/makeDisableAppsWithAddonsCallback.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; -import type { LicenseImp } from '@rocket.chat/license'; - -import type { Apps } from '../../apps'; - -type OnModuleCallbackParameter = Parameters[0]>[0]; - -export const makeDisableAppsWithAddonsCallback = (deps: { Apps: typeof Apps }) => - async function disableAppsWithAddonsCallback({ module, external, valid }: OnModuleCallbackParameter) { - if (!external || valid) return; - - const enabledApps = await deps.Apps.installedApps({ enabled: true }); - - if (!enabledApps) return; - - await Promise.all( - enabledApps.map(async (app) => { - if (app.getInfo().addon !== module) return; - - return deps.Apps.getManager()?.disable(app.getID(), AppStatus.DISABLED, false); - }), - ); - }; diff --git a/apps/meteor/ee/server/startup/apps.ts b/apps/meteor/ee/server/startup/apps.ts index f76e4294383b..b254689344b5 100644 --- a/apps/meteor/ee/server/startup/apps.ts +++ b/apps/meteor/ee/server/startup/apps.ts @@ -2,7 +2,7 @@ import { License } from '@rocket.chat/license'; import { Meteor } from 'meteor/meteor'; import { Apps } from '../apps'; -import { makeDisableAppsWithAddonsCallback } from '../lib/apps/makeDisableAppsWithAddonsCallback'; +import { disableAppsWithAddonsCallback } from '../lib/apps/disableAppsWithAddonsCallback'; Meteor.startup(() => { async function migratePrivateAppsCallback() { @@ -15,5 +15,5 @@ Meteor.startup(() => { License.onRemoveLicense(migratePrivateAppsCallback); // Disable apps that depend on add-ons (external modules) if they are invalidated - License.onModule(makeDisableAppsWithAddonsCallback({ Apps })); + License.onModule(disableAppsWithAddonsCallback); }); diff --git a/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts b/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts index 5dbde66da537..d27a99cf5ce4 100644 --- a/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/apps/disableAppsWithAddonsCallback.spec.ts @@ -1,9 +1,30 @@ import { expect, spy } from 'chai'; +import proxyquire from 'proxyquire'; import type { AppServerOrchestrator } from '../../../../../../ee/server/apps/orchestrator'; -import { makeDisableAppsWithAddonsCallback } from '../../../../../../ee/server/lib/apps/makeDisableAppsWithAddonsCallback'; + +const { _disableAppsWithAddonsCallback } = proxyquire + .noCallThru() + .load('../../../../../../ee/server/lib/apps/disableAppsWithAddonsCallback', { + '../../apps': {}, + '../../../../server/lib/sendMessagesToAdmins': { sendMessagesToAdmins: () => undefined }, + '../../../../server/lib/i18n': { + i18n: { t: () => undefined }, + }, + }); + +/** + * I've used named "empty" functions to spy on as it is easier to + * troubleshoot if the assertion fails. + * If we use `spy()` instead, there is no clear indication on the + * error message which of the spy assertions failed + */ describe('disableAppsWithAddonsCallback', () => { + function sendMessagesToAdmins(): any { + return undefined; + } + it('should not execute anything if not external module', async () => { function installedApps() { return []; @@ -13,19 +34,19 @@ describe('disableAppsWithAddonsCallback', () => { return undefined; } + const mockManager = { + disable: spy(getManagerDisable), + }; + const AppsMock = { installedApps: spy(installedApps), - getManager: () => ({ - disable: spy(getManagerDisable), - }), + getManager: () => mockManager, } as unknown as AppServerOrchestrator; - const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); - - await disableAppsWithAddonsCallback({ module: 'auditing', external: false, valid: true }); + await _disableAppsWithAddonsCallback({ Apps: AppsMock, sendMessagesToAdmins }, { module: 'auditing', external: false, valid: true }); expect(AppsMock.installedApps).to.not.have.been.called(); - expect(AppsMock.getManager()!.disable).to.not.have.been.called(); + expect(AppsMock.getManager()?.disable).to.not.have.been.called(); }); it('should not execute anything if module is external and valid', async () => { @@ -37,19 +58,19 @@ describe('disableAppsWithAddonsCallback', () => { return undefined; } + const mockManager = { + disable: spy(getManagerDisable), + }; + const AppsMock = { installedApps: spy(installedApps), - getManager: () => ({ - disable: spy(getManagerDisable), - }), + getManager: () => mockManager, } as unknown as AppServerOrchestrator; - const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); - - await disableAppsWithAddonsCallback({ module: 'auditing', external: true, valid: true }); + await _disableAppsWithAddonsCallback({ Apps: AppsMock, sendMessagesToAdmins }, { module: 'auditing', external: true, valid: true }); expect(AppsMock.installedApps).to.not.have.been.called(); - expect(AppsMock.getManager()!.disable).to.not.have.been.called(); + expect(AppsMock.getManager()?.disable).to.not.have.been.called(); }); it('should not throw if there are no apps installed that are enabled', async () => { @@ -61,19 +82,21 @@ describe('disableAppsWithAddonsCallback', () => { return undefined; } + const mockManager = { + disable: spy(getManagerDisable), + }; + const AppsMock = { installedApps: spy(installedApps), - getManager: () => ({ - disable: spy(getManagerDisable), - }), + getManager: () => mockManager, } as unknown as AppServerOrchestrator; - const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); - - await expect(disableAppsWithAddonsCallback({ module: 'auditing', external: true, valid: false })).to.not.eventually.be.rejected; + await expect( + _disableAppsWithAddonsCallback({ Apps: AppsMock, sendMessagesToAdmins }, { module: 'auditing', external: true, valid: false }), + ).to.not.eventually.be.rejected; expect(AppsMock.installedApps).to.have.been.called(); - expect(AppsMock.getManager()!.disable).to.not.have.been.called(); + expect(AppsMock.getManager()?.disable).to.not.have.been.called(); }); it('should only disable apps that require addons', async () => { @@ -81,12 +104,99 @@ describe('disableAppsWithAddonsCallback', () => { return [ { getInfo: () => ({}), + getName: () => 'Test App Without Addon', + getID() { + return 'test-app-without-addon'; + }, + }, + { + getInfo: () => ({ addon: 'chat.rocket.test-addon' }), + getName: () => 'Test App WITH Addon', + getID() { + return 'test-app-with-addon'; + }, + }, + ]; + } + + function getManagerDisable() { + return undefined; + } + + const mockManager = { + disable: spy(getManagerDisable), + }; + + const AppsMock = { + installedApps: spy(installedApps), + getManager: () => mockManager, + } as unknown as AppServerOrchestrator; + + await expect( + _disableAppsWithAddonsCallback( + { Apps: AppsMock, sendMessagesToAdmins }, + { module: 'chat.rocket.test-addon', external: true, valid: false }, + ), + ).to.not.eventually.be.rejected; + + expect(AppsMock.installedApps).to.have.been.called(); + expect(AppsMock.getManager()?.disable).to.have.been.called.once; + expect(AppsMock.getManager()?.disable).to.have.been.called.with('test-app-with-addon'); + }); + + it('should not send messages to admins if no app was disabled', async () => { + function installedApps() { + return [ + { + getInfo: () => ({}), + getName: () => 'Test App Without Addon', + getID() { + return 'test-app-without-addon'; + }, + }, + ]; + } + + function getManagerDisable() { + return undefined; + } + + const mockManager = { + disable: spy(getManagerDisable), + }; + + const AppsMock = { + installedApps: spy(installedApps), + getManager: () => mockManager, + } as unknown as AppServerOrchestrator; + + const sendMessagesToAdminsSpy = spy(sendMessagesToAdmins); + + await expect( + _disableAppsWithAddonsCallback( + { Apps: AppsMock, sendMessagesToAdmins: sendMessagesToAdminsSpy }, + { module: 'chat.rocket.test-addon', external: true, valid: false }, + ), + ).to.not.eventually.be.rejected; + + expect(AppsMock.installedApps).to.have.been.called(); + expect(AppsMock.getManager()?.disable).to.not.have.been.called(); + expect(sendMessagesToAdminsSpy).to.not.have.been.called(); + }); + + it('should send messages to admins if some app has been disabled', async () => { + function installedApps() { + return [ + { + getInfo: () => ({}), + getName: () => 'Test App Without Addon', getID() { return 'test-app-without-addon'; }, }, { getInfo: () => ({ addon: 'chat.rocket.test-addon' }), + getName: () => 'Test App WITH Addon', getID() { return 'test-app-with-addon'; }, @@ -107,13 +217,17 @@ describe('disableAppsWithAddonsCallback', () => { getManager: () => mockManager, } as unknown as AppServerOrchestrator; - const disableAppsWithAddonsCallback = makeDisableAppsWithAddonsCallback({ Apps: AppsMock }); + const sendMessagesToAdminsSpy = spy(sendMessagesToAdmins); - await expect(disableAppsWithAddonsCallback({ module: 'chat.rocket.test-addon', external: true, valid: false })).to.not.eventually.be - .rejected; + await expect( + _disableAppsWithAddonsCallback( + { Apps: AppsMock, sendMessagesToAdmins: sendMessagesToAdminsSpy }, + { module: 'chat.rocket.test-addon', external: true, valid: false }, + ), + ).to.not.eventually.be.rejected; expect(AppsMock.installedApps).to.have.been.called(); - expect(AppsMock.getManager()!.disable).to.have.been.called.once; - expect(AppsMock.getManager()!.disable).to.have.been.called.with('test-app-with-addon'); + expect(AppsMock.getManager()?.disable).to.have.been.called.once; + expect(sendMessagesToAdminsSpy).to.have.been.called(); }); }); diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index c871ce50e773..7e310b3a27e7 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -529,6 +529,8 @@ "App_Info": "App Info", "App_Information": "App Information", "Apps_context_enterprise": "Enterprise", + "App_has_been_disabled_addon_message_one": "The app {{appNames}} has been disabled because of an invalid add-on. A valid add-on subscription is required to re-enable it", + "App_has_been_disabled_addon_message_other": "The apps {{appNames}} have been disabled because of invalid add-ons. A valid add-on subscription is required to re-enable them", "App_Installation": "App Installation", "App_Installation_Deprecation_Title": "Deprecation Warning", "App_Installation_Deprecation": "Install apps from URL is deprecated and will be removed in the next major release.", From 374a43dfdd2e9df0e886fef1a6e5864490b44512 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 18 Oct 2024 09:28:22 -0300 Subject: [PATCH 23/25] Add addon property to App type for frontend usage --- packages/core-typings/src/Apps.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core-typings/src/Apps.ts b/packages/core-typings/src/Apps.ts index 3d0afce9d12c..88abc86a091c 100644 --- a/packages/core-typings/src/Apps.ts +++ b/packages/core-typings/src/Apps.ts @@ -1,4 +1,5 @@ import type { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; +import { ExternalModuleName } from './license'; export type AppScreenshot = { id: string; @@ -92,6 +93,7 @@ export type App = { categories: string[]; version: string; versionIncompatible?: boolean; + addon: ExternalModuleName; price: number; purchaseType: PurchaseType; pricingPlans: AppPricingPlan[]; From 75a11c61d1c43d0b2287550ac466f19407124a67 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 18 Oct 2024 10:16:13 -0300 Subject: [PATCH 24/25] Oops --- packages/core-typings/src/Apps.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-typings/src/Apps.ts b/packages/core-typings/src/Apps.ts index 88abc86a091c..ad8867264f15 100644 --- a/packages/core-typings/src/Apps.ts +++ b/packages/core-typings/src/Apps.ts @@ -93,7 +93,7 @@ export type App = { categories: string[]; version: string; versionIncompatible?: boolean; - addon: ExternalModuleName; + addon?: ExternalModuleName; price: number; purchaseType: PurchaseType; pricingPlans: AppPricingPlan[]; From c0d49fb9c0fbb5da2dcc21465334df5dd84f4ea5 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 18 Oct 2024 11:00:18 -0300 Subject: [PATCH 25/25] Fix lint --- packages/core-typings/src/Apps.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core-typings/src/Apps.ts b/packages/core-typings/src/Apps.ts index ad8867264f15..d1b0ddac4a56 100644 --- a/packages/core-typings/src/Apps.ts +++ b/packages/core-typings/src/Apps.ts @@ -1,5 +1,6 @@ import type { AppStatus } from '@rocket.chat/apps-engine/definition/AppStatus'; -import { ExternalModuleName } from './license'; + +import type { ExternalModuleName } from './license'; export type AppScreenshot = { id: string;