[AC-1479][BEEEP] Refactor ConfigService to improve observable usage (#5602)

* refactor ConfigService to use observables

* make environmentService.urls a ReplaySubject

---------

Co-authored-by: Hinton <hinton@users.noreply.github.com>

apps/browser/src/background/main.background.ts

@@ -35,7 +35,6 @@ import { UserVerificationApiService } from "@bitwarden/common/auth/services/user
 import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service";
 import { AppIdService as AppIdServiceAbstraction } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction";
-import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
 import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto-function.service";
 import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto.service";
 import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
@@ -53,7 +52,6 @@ import { StateFactory } from "@bitwarden/common/platform/factories/state-factory
 import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
 import { AppIdService } from "@bitwarden/common/platform/services/app-id.service";
 import { ConfigApiService } from "@bitwarden/common/platform/services/config/config-api.service";
-import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
 import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service";
 import { ContainerService } from "@bitwarden/common/platform/services/container.service";
 import { EncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/encrypt.service.implementation";
@@ -123,6 +121,7 @@ import { flagEnabled } from "../platform/flags";
 import { UpdateBadge } from "../platform/listeners/update-badge";
 import BrowserPopoutWindowService from "../platform/popup/browser-popout-window.service";
 import { BrowserStateService as StateServiceAbstraction } from "../platform/services/abstractions/browser-state.service";
+import { BrowserConfigService } from "../platform/services/browser-config.service";
 import { BrowserCryptoService } from "../platform/services/browser-crypto.service";
 import { BrowserEnvironmentService } from "../platform/services/browser-environment.service";
 import { BrowserI18nService } from "../platform/services/browser-i18n.service";
@@ -200,7 +199,7 @@ export default class MainBackground {
   avatarUpdateService: AvatarUpdateServiceAbstraction;
   mainContextMenuHandler: MainContextMenuHandler;
   cipherContextMenuHandler: CipherContextMenuHandler;
-  configService: ConfigServiceAbstraction;
+  configService: BrowserConfigService;
   configApiService: ConfigApiServiceAbstraction;
   devicesApiService: DevicesApiServiceAbstraction;
   devicesService: DevicesServiceAbstraction;
@@ -533,12 +532,15 @@ export default class MainBackground {
       this.authService,
       this.messagingService
     );
+
     this.configApiService = new ConfigApiService(this.apiService, this.authService);
-    this.configService = new ConfigService(
+
+    this.configService = new BrowserConfigService(
       this.stateService,
       this.configApiService,
       this.authService,
-      this.environmentService
+      this.environmentService,
+      true
     );
     this.browserPopoutWindowService = new BrowserPopoutWindowService();
 
@@ -682,6 +684,7 @@ export default class MainBackground {
     await this.notificationBackground.init();
     await this.commandsBackground.init();
 
+    this.configService.init();
     this.twoFactorService.init();
 
     await this.tabsBackground.init();

apps/browser/src/background/runtime.background.ts

@@ -103,7 +103,7 @@ export default class RuntimeBackground {
             await this.main.refreshMenu();
           }, 2000);
           this.main.avatarUpdateService.loadColorFromState();
-          this.configService.fetchServerConfig();
+          this.configService.triggerServerConfigFetch();
         }
         break;
       case "openPopup":
@@ -139,7 +139,7 @@ export default class RuntimeBackground {
       case "triggerAutofillScriptInjection":
         await this.autofillService.injectAutofillScripts(
           sender,
-          await this.configService.getFeatureFlagBool(FeatureFlag.AutofillV2)
+          await this.configService.getFeatureFlag<boolean>(FeatureFlag.AutofillV2)
         );
         break;
       case "bgCollectPageDetails":

apps/browser/src/platform/services/browser-config.service.ts

@@ -1,6 +1,10 @@
-import { BehaviorSubject } from "rxjs";
+import { ReplaySubject } from "rxjs";
 
+import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
+import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction";
 import { ServerConfig } from "@bitwarden/common/platform/abstractions/config/server-config";
+import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
+import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
 
 import { browserSession, sessionSync } from "../decorators/session-sync-observable";
@@ -8,5 +12,15 @@ import { browserSession, sessionSync } from "../decorators/session-sync-observab
 @browserSession
 export class BrowserConfigService extends ConfigService {
   @sessionSync<ServerConfig>({ initializer: ServerConfig.fromJSON })
-  protected _serverConfig: BehaviorSubject<ServerConfig | null>;
+  protected _serverConfig: ReplaySubject<ServerConfig | null>;
+
+  constructor(
+    stateService: StateService,
+    configApiService: ConfigApiServiceAbstraction,
+    authService: AuthService,
+    environmentService: EnvironmentService,
+    subscribe = false
+  ) {
+    super(stateService, configApiService, authService, environmentService, subscribe);
+  }
 }

apps/browser/src/popup/services/init.service.ts

@@ -4,6 +4,7 @@ import { AbstractThemingService } from "@bitwarden/angular/services/theming/them
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService as LogServiceAbstraction } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
 
 import { BrowserStateService as StateServiceAbstraction } from "../../platform/services/abstractions/browser-state.service";
 
@@ -17,7 +18,8 @@ export class InitService {
     private popupUtilsService: PopupUtilsService,
     private stateService: StateServiceAbstraction,
     private logService: LogServiceAbstraction,
-    private themingService: AbstractThemingService
+    private themingService: AbstractThemingService,
+    private configService: ConfigService
   ) {}
 
   init() {
@@ -50,6 +52,8 @@ export class InitService {
         htmlEl.classList.add("force_redraw");
         this.logService.info("Force redraw is on");
       }
+
+      this.configService.init();
     };
   }
 }

apps/browser/src/popup/services/services.module.ts

@@ -36,7 +36,6 @@ import { AuthService } from "@bitwarden/common/auth/services/auth.service";
 import { LoginService } from "@bitwarden/common/auth/services/login.service";
 import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction";
-import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
 import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
 import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
 import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
@@ -57,6 +56,7 @@ import {
 } from "@bitwarden/common/platform/abstractions/storage.service";
 import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
 import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
+import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
 import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service";
 import { ContainerService } from "@bitwarden/common/platform/services/container.service";
 import { SearchService } from "@bitwarden/common/services/search.service";
@@ -495,7 +495,7 @@ function getBgService<T>(service: keyof MainBackground) {
       deps: [StateServiceAbstraction, PlatformUtilsService],
     },
     {
-      provide: ConfigServiceAbstraction,
+      provide: ConfigService,
       useClass: BrowserConfigService,
       deps: [
         StateServiceAbstraction,

apps/desktop/src/app/app.component.ts

@@ -232,7 +232,7 @@ export class AppComponent implements OnInit, OnDestroy {
             break;
           case "syncCompleted":
             await this.updateAppMenu();
-            await this.configService.fetchServerConfig();
+            this.configService.triggerServerConfigFetch();
             break;
           case "openSettings":
             await this.openModal<SettingsComponent>(SettingsComponent, this.settingsRef);

apps/desktop/src/app/services/init.service.ts

@@ -11,6 +11,7 @@ import { EnvironmentService as EnvironmentServiceAbstraction } from "@bitwarden/
 import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service";
+import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
 import { ContainerService } from "@bitwarden/common/platform/services/container.service";
 import { EventUploadService } from "@bitwarden/common/services/event/event-upload.service";
 import { VaultTimeoutService } from "@bitwarden/common/services/vault-timeout/vault-timeout.service";
@@ -35,7 +36,8 @@ export class InitService {
     private cryptoService: CryptoServiceAbstraction,
     private nativeMessagingService: NativeMessagingService,
     private themingService: AbstractThemingService,
-    private encryptService: EncryptService
+    private encryptService: EncryptService,
+    private configService: ConfigService
   ) {}
 
   init() {
@@ -71,6 +73,8 @@ export class InitService {
 
       const containerService = new ContainerService(this.cryptoService, this.encryptService);
       containerService.attachToGlobal(this.win);
+
+      this.configService.init();
     };
   }
 }

apps/web/src/app/app.component.ts

@@ -138,7 +138,7 @@ export class AppComponent implements OnDestroy, OnInit {
           case "syncStarted":
             break;
           case "syncCompleted":
-            await this.configService.fetchServerConfig();
+            this.configService.triggerServerConfigFetch();
             break;
           case "upgradeOrganization": {
             const upgradeConfirmed = await this.dialogService.openSimpleDialog({

apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts

@@ -135,7 +135,7 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy
     this.loading = false;
 
     // Remove the remaining lines when the sm-ga-billing flag is deleted
-    const smBillingEnabled = await this.configService.getFeatureFlagBool(
+    const smBillingEnabled = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.SecretsManagerBilling
     );
     this.showSecretsManagerSubscribe = this.showSecretsManagerSubscribe && smBillingEnabled;

apps/web/src/app/billing/settings/add-credit.component.ts

@@ -7,6 +7,7 @@ import {
   Output,
   ViewChild,
 } from "@angular/core";
+import { firstValueFrom } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
@@ -79,7 +80,7 @@ export class AddCreditComponent implements OnInit {
       this.email = this.subject;
       this.ppButtonCustomField = "user_id:" + this.userId;
     }
-    this.region = await this.configService.getCloudRegion();
+    this.region = await firstValueFrom(this.configService.cloudRegion$);
     this.ppButtonCustomField += ",account_credit:1";
     this.ppButtonCustomField += `,region:${this.region}`;
     this.returnUrl = window.location.href;

apps/web/src/app/billing/settings/organization-plans.component.ts

@@ -169,7 +169,7 @@ export class OrganizationPlansComponent implements OnInit, OnDestroy {
         this.singleOrgPolicyAppliesToActiveUser = policyAppliesToActiveUser;
       });
 
-    this.showSecretsManagerSubscribe = await this.configService.getFeatureFlagBool(
+    this.showSecretsManagerSubscribe = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.SecretsManagerBilling,
       false
     );

apps/web/src/app/components/environment-selector/environment-selector.component.ts

@@ -25,7 +25,7 @@ export class EnvironmentSelectorComponent implements OnInit {
   routeAndParams: string;
 
   async ngOnInit() {
-    this.euServerFlagEnabled = await this.configService.getFeatureFlagBool(
+    this.euServerFlagEnabled = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.DisplayEuEnvironmentFlag
     );
     const domain = Utils.getDomain(window.location.href);

apps/web/src/app/core/init.service.ts

@@ -13,6 +13,7 @@ import {
 } from "@bitwarden/common/platform/abstractions/environment.service";
 import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service";
+import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
 import { ContainerService } from "@bitwarden/common/platform/services/container.service";
 import { EventUploadService } from "@bitwarden/common/services/event/event-upload.service";
 import { VaultTimeoutService } from "@bitwarden/common/services/vault-timeout/vault-timeout.service";
@@ -32,7 +33,8 @@ export class InitService {
     private stateService: StateServiceAbstraction,
     private cryptoService: CryptoServiceAbstraction,
     private themingService: AbstractThemingService,
-    private encryptService: EncryptService
+    private encryptService: EncryptService,
+    private configService: ConfigService
   ) {}
 
   init() {
@@ -57,6 +59,8 @@ export class InitService {
       await this.themingService.monitorThemeChanges();
       const containerService = new ContainerService(this.cryptoService, this.encryptService);
       containerService.attachToGlobal(this.win);
+
+      this.configService.init();
     };
   }
 }

bitwarden_license/bit-web/src/app/auth/sso/sso.component.ts

@@ -235,7 +235,7 @@ export class SsoComponent implements OnInit, OnDestroy {
       )
       .subscribe();
 
-    const tdeFeatureFlag = await this.configService.getFeatureFlagBool(
+    const tdeFeatureFlag = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.TrustedDeviceEncryption
     );
 

libs/angular/src/auth/components/environment-selector.component.ts

@@ -89,7 +89,7 @@ export class EnvironmentSelectorComponent implements OnInit, OnDestroy {
 
   async updateEnvironmentInfo() {
     this.selectedEnvironment = this.environmentService.selectedRegion;
-    this.euServerFlagEnabled = await this.configService.getFeatureFlagBool(
+    this.euServerFlagEnabled = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.DisplayEuEnvironmentFlag
     );
   }

libs/angular/src/auth/components/sso.component.spec.ts

@@ -331,7 +331,7 @@ describe("SsoComponent", () => {
 
     describe("Trusted Device Encryption scenarios", () => {
       beforeEach(() => {
-        mockConfigService.getFeatureFlagBool.mockResolvedValue(true); // TDE enabled
+        mockConfigService.getFeatureFlag.mockResolvedValue(true); // TDE enabled
       });
 
       describe("Given Trusted Device Encryption is enabled and user needs to set a master password", () => {

libs/angular/src/auth/components/sso.component.ts

@@ -242,7 +242,7 @@ export class SsoComponent {
   private async isTrustedDeviceEncEnabled(
     trustedDeviceOption: TrustedDeviceUserDecryptionOption
   ): Promise<boolean> {
-    const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlagBool(
+    const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.TrustedDeviceEncryption
     );
 

libs/angular/src/auth/components/two-factor.component.spec.ts

@@ -376,7 +376,7 @@ describe("TwoFactorComponent", () => {
 
       describe("Trusted Device Encryption scenarios", () => {
         beforeEach(() => {
-          mockConfigService.getFeatureFlagBool.mockResolvedValue(true);
+          mockConfigService.getFeatureFlag.mockResolvedValue(true);
         });
 
         describe("Given Trusted Device Encryption is enabled and user needs to set a master password", () => {

libs/angular/src/auth/components/two-factor.component.ts

@@ -257,7 +257,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI
     trustedDeviceOption: TrustedDeviceUserDecryptionOption
   ): Promise<boolean> {
     const ssoTo2faFlowActive = this.route.snapshot.queryParamMap.get("sso") === "true";
-    const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlagBool(
+    const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlag<boolean>(
       FeatureFlag.TrustedDeviceEncryption
     );
 

libs/angular/src/directives/if-feature.directive.spec.ts

@@ -3,7 +3,7 @@ import { ComponentFixture, TestBed } from "@angular/core/testing";
 import { By } from "@angular/platform-browser";
 import { mock, MockProxy } from "jest-mock-extended";
 
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { FeatureFlag, FeatureFlagValue } from "@bitwarden/common/enums/feature-flag.enum";
 import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 
@@ -41,21 +41,12 @@ describe("IfFeatureDirective", () => {
   let content: HTMLElement;
   let mockConfigService: MockProxy<ConfigServiceAbstraction>;
 
-  const mockConfigFlagValue = (flag: FeatureFlag, flagValue: any) => {
-    if (typeof flagValue === "boolean") {
-      mockConfigService.getFeatureFlagBool.mockImplementation((f, defaultValue = false) =>
+  const mockConfigFlagValue = (flag: FeatureFlag, flagValue: FeatureFlagValue) => {
+    mockConfigService.getFeatureFlag.mockImplementation((f, defaultValue) =>
       flag == f ? Promise.resolve(flagValue) : Promise.resolve(defaultValue)
     );
-    } else if (typeof flagValue === "string") {
-      mockConfigService.getFeatureFlagString.mockImplementation((f, defaultValue = "") =>
-        flag == f ? Promise.resolve(flagValue) : Promise.resolve(defaultValue)
-      );
-    } else if (typeof flagValue === "number") {
-      mockConfigService.getFeatureFlagNumber.mockImplementation((f, defaultValue = 0) =>
-        flag == f ? Promise.resolve(flagValue) : Promise.resolve(defaultValue)
-      );
-    }
   };
+
   const queryContent = (testId: string) =>
     fixture.debugElement.query(By.css(`[data-testid="${testId}"]`))?.nativeElement;
 
@@ -126,7 +117,7 @@ describe("IfFeatureDirective", () => {
   });
 
   it("hides content when the directive throws an unexpected exception", async () => {
-    mockConfigService.getFeatureFlagBool.mockImplementation(() => Promise.reject("Some error"));
+    mockConfigService.getFeatureFlag.mockImplementation(() => Promise.reject("Some error"));
     fixture.detectChanges();
     await fixture.whenStable();
 

libs/angular/src/directives/if-feature.directive.ts

@@ -1,12 +1,9 @@
 import { Directive, Input, OnInit, TemplateRef, ViewContainerRef } from "@angular/core";
 
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { FeatureFlag, FeatureFlagValue } from "@bitwarden/common/enums/feature-flag.enum";
 import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 
-// Replace this with a type safe lookup of the feature flag values in PM-2282
-type FlagValue = boolean | number | string;
-
 /**
  * Directive that conditionally renders the element when the feature flag is enabled and/or
  * matches the value specified by {@link appIfFeatureValue}.
@@ -26,7 +23,7 @@ export class IfFeatureDirective implements OnInit {
    * Optional value to compare against the value of the feature flag in the config service.
    * @default true
    */
-  @Input() appIfFeatureValue: FlagValue = true;
+  @Input() appIfFeatureValue: FeatureFlagValue = true;
 
   private hasView = false;
 
@@ -39,15 +36,7 @@ export class IfFeatureDirective implements OnInit {
 
   async ngOnInit() {
     try {
-      let flagValue: FlagValue;
-
-      if (typeof this.appIfFeatureValue === "boolean") {
-        flagValue = await this.configService.getFeatureFlagBool(this.appIfFeature);
-      } else if (typeof this.appIfFeatureValue === "number") {
-        flagValue = await this.configService.getFeatureFlagNumber(this.appIfFeature);
-      } else if (typeof this.appIfFeatureValue === "string") {
-        flagValue = await this.configService.getFeatureFlagString(this.appIfFeature);
-      }
+      const flagValue = await this.configService.getFeatureFlag(this.appIfFeature);
 
       if (this.appIfFeatureValue === flagValue) {
         if (!this.hasView) {

libs/angular/src/guard/feature-flag.guard.spec.ts

@@ -30,15 +30,15 @@ describe("canAccessFeature", () => {
 
     // Mock the correct getter based on the type of flagValue; also mock default values if one is not provided
     if (typeof flagValue === "boolean") {
-      mockConfigService.getFeatureFlagBool.mockImplementation((flag, defaultValue = false) =>
+      mockConfigService.getFeatureFlag.mockImplementation((flag, defaultValue = false) =>
         flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue)
       );
     } else if (typeof flagValue === "string") {
-      mockConfigService.getFeatureFlagString.mockImplementation((flag, defaultValue = "") =>
+      mockConfigService.getFeatureFlag.mockImplementation((flag, defaultValue = "") =>
         flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue)
       );
     } else if (typeof flagValue === "number") {
-      mockConfigService.getFeatureFlagNumber.mockImplementation((flag, defaultValue = 0) =>
+      mockConfigService.getFeatureFlag.mockImplementation((flag, defaultValue = 0) =>
         flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue)
       );
     }
@@ -143,7 +143,7 @@ describe("canAccessFeature", () => {
   it("fails to navigate when the config service throws an unexpected exception", async () => {
     const { router } = setup(canAccessFeature(testFlag), true);
 
-    mockConfigService.getFeatureFlagBool.mockImplementation(() => Promise.reject("Some error"));
+    mockConfigService.getFeatureFlag.mockImplementation(() => Promise.reject("Some error"));
 
     await router.navigate([featureRoute]);
 

libs/angular/src/guard/feature-flag.guard.ts

@@ -29,16 +29,8 @@ export const canAccessFeature = (
     const i18nService = inject(I18nService);
     const logService = inject(LogService);
 
-    let flagValue: FlagValue;
-
     try {
-      if (typeof requiredFlagValue === "boolean") {
-        flagValue = await configService.getFeatureFlagBool(featureFlag);
-      } else if (typeof requiredFlagValue === "number") {
-        flagValue = await configService.getFeatureFlagNumber(featureFlag);
-      } else if (typeof requiredFlagValue === "string") {
-        flagValue = await configService.getFeatureFlagString(featureFlag);
-      }
+      const flagValue = await configService.getFeatureFlag(featureFlag);
 
       if (flagValue === requiredFlagValue) {
         return true;

libs/angular/src/services/jslib-services.module.ts

@@ -640,7 +640,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
       useClass: SyncNotifierService,
     },
     {
-      provide: ConfigServiceAbstraction,
+      provide: ConfigService,
       useClass: ConfigService,
       deps: [
         StateServiceAbstraction,
@@ -649,6 +649,10 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
         EnvironmentServiceAbstraction,
       ],
     },
+    {
+      provide: ConfigServiceAbstraction,
+      useExisting: ConfigService,
+    },
     {
       provide: ConfigApiServiceAbstraction,
       useClass: ConfigApiService,

libs/common/src/enums/feature-flag.enum.ts

@@ -5,3 +5,6 @@ export enum FeatureFlag {
   AutofillV2 = "autofill-v2",
   SecretsManagerBilling = "sm-ga-billing",
 }
+
+// Replace this with a type safe lookup of the feature flag values in PM-2282
+export type FeatureFlagValue = number | string | boolean;

libs/common/src/platform/abstractions/config/config.service.abstraction.ts

@@ -1,14 +1,26 @@
 import { Observable } from "rxjs";
 
 import { FeatureFlag } from "../../../enums/feature-flag.enum";
+import { Region } from "../environment.service";
 
 import { ServerConfig } from "./server-config";
 
 export abstract class ConfigServiceAbstraction {
   serverConfig$: Observable<ServerConfig | null>;
-  fetchServerConfig: () => Promise<ServerConfig>;
-  getFeatureFlagBool: (key: FeatureFlag, defaultValue?: boolean) => Promise<boolean>;
-  getFeatureFlagString: (key: FeatureFlag, defaultValue?: string) => Promise<string>;
-  getFeatureFlagNumber: (key: FeatureFlag, defaultValue?: number) => Promise<number>;
-  getCloudRegion: (defaultValue?: string) => Promise<string>;
+  cloudRegion$: Observable<Region>;
+  getFeatureFlag$: <T extends boolean | number | string>(
+    key: FeatureFlag,
+    defaultValue?: T
+  ) => Observable<T>;
+  getFeatureFlag: <T extends boolean | number | string>(
+    key: FeatureFlag,
+    defaultValue?: T
+  ) => Promise<T>;
+
+  /**
+   * Force ConfigService to fetch an updated config from the server and emit it from serverConfig$
+   * @deprecated The service implementation should subscribe to an observable and use that to trigger a new fetch from
+   * server instead
+   */
+  triggerServerConfigFetch: () => void;
 }

libs/common/src/platform/services/config/config.service.spec.ts

@@ -0,0 +1,174 @@
+import { MockProxy, mock } from "jest-mock-extended";
+import { ReplaySubject, skip, take } from "rxjs";
+
+import { AuthService } from "../../../auth/abstractions/auth.service";
+import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
+import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction";
+import { ServerConfig } from "../../abstractions/config/server-config";
+import { EnvironmentService } from "../../abstractions/environment.service";
+import { StateService } from "../../abstractions/state.service";
+import { ServerConfigData } from "../../models/data/server-config.data";
+import {
+  EnvironmentServerConfigResponse,
+  ServerConfigResponse,
+  ThirdPartyServerConfigResponse,
+} from "../../models/response/server-config.response";
+
+import { ConfigService } from "./config.service";
+
+describe("ConfigService", () => {
+  let stateService: MockProxy<StateService>;
+  let configApiService: MockProxy<ConfigApiServiceAbstraction>;
+  let authService: MockProxy<AuthService>;
+  let environmentService: MockProxy<EnvironmentService>;
+
+  let serverResponseCount: number; // increments to track distinct responses received from server
+
+  // Observables will start emitting as soon as this is created, so only create it
+  // after everything is mocked
+  const configServiceFactory = () => {
+    const configService = new ConfigService(
+      stateService,
+      configApiService,
+      authService,
+      environmentService
+    );
+    configService.init();
+    return configService;
+  };
+
+  beforeEach(() => {
+    stateService = mock();
+    configApiService = mock();
+    authService = mock();
+    environmentService = mock();
+    environmentService.urls = new ReplaySubject<void>(1);
+
+    serverResponseCount = 1;
+    configApiService.get.mockImplementation(() =>
+      Promise.resolve(serverConfigResponseFactory("server" + serverResponseCount++))
+    );
+
+    jest.useFakeTimers();
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
+  it("Loads config from storage", (done) => {
+    const storedConfigData = serverConfigDataFactory("storedConfig");
+    stateService.getServerConfig.mockResolvedValueOnce(storedConfigData);
+
+    const configService = configServiceFactory();
+
+    configService.serverConfig$.pipe(take(1)).subscribe((config) => {
+      expect(config).toEqual(new ServerConfig(storedConfigData));
+      expect(stateService.getServerConfig).toHaveBeenCalledTimes(1);
+      expect(stateService.setServerConfig).not.toHaveBeenCalled();
+      done();
+    });
+  });
+
+  describe("Fetches config from server", () => {
+    beforeEach(() => {
+      stateService.getServerConfig.mockResolvedValueOnce(null);
+    });
+
+    it.each<number | jest.DoneCallback>([1, 2, 3])(
+      "after %p hour/s",
+      (hours: number, done: jest.DoneCallback) => {
+        const configService = configServiceFactory();
+
+        // skip initial load from storage, plus previous hours (if any)
+        configService.serverConfig$.pipe(skip(hours), take(1)).subscribe((config) => {
+          try {
+            expect(config.gitHash).toEqual("server" + hours);
+            expect(configApiService.get).toHaveBeenCalledTimes(hours);
+            done();
+          } catch (e) {
+            done(e);
+          }
+        });
+
+        const oneHourInMs = 1000 * 3600;
+        jest.advanceTimersByTime(oneHourInMs * hours + 1);
+      }
+    );
+
+    it("when environment URLs change", (done) => {
+      const configService = configServiceFactory();
+
+      // skip initial load from storage
+      configService.serverConfig$.pipe(skip(1), take(1)).subscribe((config) => {
+        try {
+          expect(config.gitHash).toEqual("server1");
+          done();
+        } catch (e) {
+          done(e);
+        }
+      });
+
+      (environmentService.urls as ReplaySubject<void>).next();
+    });
+
+    it("when triggerServerConfigFetch() is called", (done) => {
+      const configService = configServiceFactory();
+
+      // skip initial load from storage
+      configService.serverConfig$.pipe(skip(1), take(1)).subscribe((config) => {
+        try {
+          expect(config.gitHash).toEqual("server1");
+          done();
+        } catch (e) {
+          done(e);
+        }
+      });
+
+      configService.triggerServerConfigFetch();
+    });
+  });
+
+  it("Saves server config to storage when the user is logged in", (done) => {
+    stateService.getServerConfig.mockResolvedValueOnce(null);
+    authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked);
+    const configService = configServiceFactory();
+
+    // skip initial load from storage
+    configService.serverConfig$.pipe(skip(1), take(1)).subscribe(() => {
+      try {
+        expect(stateService.setServerConfig).toHaveBeenCalledWith(
+          expect.objectContaining({ gitHash: "server1" })
+        );
+        done();
+      } catch (e) {
+        done(e);
+      }
+    });
+
+    configService.triggerServerConfigFetch();
+  });
+});
+
+function serverConfigDataFactory(gitHash: string) {
+  return new ServerConfigData(serverConfigResponseFactory(gitHash));
+}
+
+function serverConfigResponseFactory(gitHash: string) {
+  return new ServerConfigResponse({
+    version: "myConfigVersion",
+    gitHash: gitHash,
+    server: new ThirdPartyServerConfigResponse({
+      name: "myThirdPartyServer",
+      url: "www.example.com",
+    }),
+    environment: new EnvironmentServerConfigResponse({
+      vault: "vault.example.com",
+    }),
+    featureStates: {
+      feat1: "off",
+      feat2: "on",
+      feat3: "off",
+    },
+  });
+}

libs/common/src/platform/services/config/config.service.ts

@@ -1,103 +1,106 @@
-import { BehaviorSubject, concatMap, from, timer } from "rxjs";
+import {
+  ReplaySubject,
+  Subject,
+  concatMap,
+  delayWhen,
+  filter,
+  firstValueFrom,
+  from,
+  map,
+  merge,
+  timer,
+} from "rxjs";
 
 import { AuthService } from "../../../auth/abstractions/auth.service";
 import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
-import { FeatureFlag } from "../../../enums/feature-flag.enum";
+import { FeatureFlag, FeatureFlagValue } from "../../../enums/feature-flag.enum";
 import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction";
 import { ConfigServiceAbstraction } from "../../abstractions/config/config.service.abstraction";
 import { ServerConfig } from "../../abstractions/config/server-config";
-import { EnvironmentService } from "../../abstractions/environment.service";
+import { EnvironmentService, Region } from "../../abstractions/environment.service";
 import { StateService } from "../../abstractions/state.service";
 import { ServerConfigData } from "../../models/data/server-config.data";
 
+const ONE_HOUR_IN_MILLISECONDS = 1000 * 3600;
+
 export class ConfigService implements ConfigServiceAbstraction {
-  protected _serverConfig = new BehaviorSubject<ServerConfig | null>(null);
+  protected _serverConfig = new ReplaySubject<ServerConfig | null>(1);
   serverConfig$ = this._serverConfig.asObservable();
+  private _forceFetchConfig = new Subject<void>();
+  private inited = false;
+
+  cloudRegion$ = this.serverConfig$.pipe(
+    map((config) => config?.environment?.cloudRegion ?? Region.US)
+  );
 
   constructor(
     private stateService: StateService,
     private configApiService: ConfigApiServiceAbstraction,
     private authService: AuthService,
-    private environmentService: EnvironmentService
-  ) {
-    // Re-fetch the server config every hour
-    timer(0, 1000 * 3600)
-      .pipe(concatMap(() => from(this.fetchServerConfig())))
-      .subscribe((serverConfig) => {
-        this._serverConfig.next(serverConfig);
-      });
+    private environmentService: EnvironmentService,
 
-    this.environmentService.urls.subscribe(() => {
-      this.fetchServerConfig();
-    });
-  }
+    // Used to avoid duplicate subscriptions, e.g. in browser between the background and popup
+    private subscribe = true
+  ) {}
 
-  async fetchServerConfig(): Promise<ServerConfig> {
-    try {
-      const response = await this.configApiService.get();
-
-      if (response == null) {
+  init() {
+    if (!this.subscribe || this.inited) {
       return;
     }
 
-      const data = new ServerConfigData(response);
-      const serverConfig = new ServerConfig(data);
-      this._serverConfig.next(serverConfig);
+    // Get config from storage on initial load
+    const fromStorage = from(this.stateService.getServerConfig()).pipe(
+      map((data) => (data == null ? null : new ServerConfig(data)))
+    );
+
+    fromStorage.subscribe((config) => this._serverConfig.next(config));
+
+    // Fetch config from server
+    // If you need to fetch a new config when an event occurs, add an observable that emits on that event here
+    merge(
+      timer(ONE_HOUR_IN_MILLISECONDS, ONE_HOUR_IN_MILLISECONDS), // after 1 hour, then every hour
+      this.environmentService.urls, // when environment URLs change (including when app is started)
+      this._forceFetchConfig // manual
+    )
+      .pipe(
+        delayWhen(() => fromStorage), // wait until storage has emitted first to avoid a race condition
+        concatMap(() => this.configApiService.get()),
+        filter((response) => response != null),
+        map((response) => new ServerConfigData(response)),
+        delayWhen((data) => this.saveConfig(data)),
+        map((data) => new ServerConfig(data))
+      )
+      .subscribe((config) => this._serverConfig.next(config));
+
+    this.inited = true;
+  }
+
+  getFeatureFlag$<T extends FeatureFlagValue>(key: FeatureFlag, defaultValue?: T) {
+    return this.serverConfig$.pipe(
+      map((serverConfig) => {
+        if (serverConfig?.featureStates == null || serverConfig.featureStates[key] == null) {
+          return defaultValue;
+        }
+
+        return serverConfig.featureStates[key] as T;
+      })
+    );
+  }
+
+  async getFeatureFlag<T extends FeatureFlagValue>(key: FeatureFlag, defaultValue?: T) {
+    return await firstValueFrom(this.getFeatureFlag$(key, defaultValue));
+  }
+
+  triggerServerConfigFetch() {
+    this._forceFetchConfig.next();
+  }
+
+  private async saveConfig(data: ServerConfigData) {
+    if ((await this.authService.getAuthStatus()) === AuthenticationStatus.LoggedOut) {
+      return;
+    }
 
-      const userAuthStatus = await this.authService.getAuthStatus();
-      if (userAuthStatus !== AuthenticationStatus.LoggedOut) {
-        // Store the config for offline use if the user is logged in
     await this.stateService.setServerConfig(data);
     this.environmentService.setCloudWebVaultUrl(data.environment?.cloudRegion);
   }
-      // Always return new server config from server to calling method
-      // to ensure up to date information
-      // This change is specifically for the getFeatureFlag > buildServerConfig flow
-      // for locked or logged in users.
-      return serverConfig;
-    } catch {
-      return null;
-    }
-  }
-
-  async getFeatureFlagBool(key: FeatureFlag, defaultValue = false): Promise<boolean> {
-    return await this.getFeatureFlag(key, defaultValue);
-  }
-
-  async getFeatureFlagString(key: FeatureFlag, defaultValue = ""): Promise<string> {
-    return await this.getFeatureFlag(key, defaultValue);
-  }
-
-  async getFeatureFlagNumber(key: FeatureFlag, defaultValue = 0): Promise<number> {
-    return await this.getFeatureFlag(key, defaultValue);
-  }
-
-  async getCloudRegion(defaultValue = "US"): Promise<string> {
-    const serverConfig = await this.buildServerConfig();
-    return serverConfig.environment?.cloudRegion ?? defaultValue;
-  }
-
-  private async getFeatureFlag<T>(key: FeatureFlag, defaultValue: T): Promise<T> {
-    const serverConfig = await this.buildServerConfig();
-    if (
-      serverConfig == null ||
-      serverConfig.featureStates == null ||
-      serverConfig.featureStates[key] == null
-    ) {
-      return defaultValue;
-    }
-    return serverConfig.featureStates[key] as T;
-  }
-
-  private async buildServerConfig(): Promise<ServerConfig> {
-    const data = await this.stateService.getServerConfig();
-    const domain = data ? new ServerConfig(data) : this._serverConfig.getValue();
-
-    if (domain == null || !domain.isValid() || domain.expiresSoon()) {
-      const value = await this.fetchServerConfig();
-      return value ?? domain;
-    }
-
-    return domain;
-  }
 }

libs/common/src/platform/services/environment.service.ts

@@ -1,4 +1,4 @@
-import { concatMap, Observable, Subject } from "rxjs";
+import { concatMap, Observable, ReplaySubject } from "rxjs";
 
 import { EnvironmentUrls } from "../../auth/models/domain/environment-urls";
 import {
@@ -9,7 +9,7 @@ import {
 import { StateService } from "../abstractions/state.service";
 
 export class EnvironmentService implements EnvironmentServiceAbstraction {
-  private readonly urlsSubject = new Subject<void>();
+  private readonly urlsSubject = new ReplaySubject<void>(1);
   urls: Observable<void> = this.urlsSubject.asObservable();
   selectedRegion?: Region;
   initialized = false;