[EC-522] Improve handling of rxjs subjects (#3772)

* [EC-522] feat: no public rxjs subjects

* [EC-522] feat: improve null handling

* [EC-552] fix: init subject with empty set instead of null

* [EC-552] fix: don't push null into account subject

* [EC-522] feat: remove null filter

.eslintrc.json

@@ -68,6 +68,7 @@
       }
     ],
     "rxjs-angular/prefer-takeuntil": "error",
+    "rxjs/no-exposed-subjects": ["error", { "allowProtected": true }],
     "no-restricted-syntax": [
       "error",
       {

apps/browser/src/decorators/session-sync-observable/browser-session.decorator.spec.ts

@@ -38,7 +38,7 @@ describe("browserSession decorator", () => {
     @browserSession
     class TestClass {
       @sessionSync({ initializer: (s: string) => s })
-      behaviorSubject = new BehaviorSubject("");
+      private behaviorSubject = new BehaviorSubject("");
 
       constructor(private stateService: StateService) {}
 

apps/browser/src/decorators/session-sync-observable/session-sync.decorator.spec.ts

@@ -7,7 +7,11 @@ describe("sessionSync decorator", () => {
   const ctor = String;
   class TestClass {
     @sessionSync({ ctor: ctor, initializer: initializer })
-    testProperty = new BehaviorSubject("");
+    private testProperty = new BehaviorSubject("");
+
+    complete() {
+      this.testProperty.complete();
+    }
   }
 
   it("should add __syncedItemKeys to prototype", () => {
@@ -19,7 +23,7 @@ describe("sessionSync decorator", () => {
         ctor: ctor,
         initializer: initializer,
       }),
-      testClass.testProperty.complete(),
+      testClass.complete(),
     ]);
   });
 });

apps/browser/src/popup/vault/vault-select.component.ts

@@ -54,12 +54,12 @@ export class VaultSelectComponent implements OnInit, OnDestroy {
   buttonRef: ElementRef<HTMLButtonElement>;
   @ViewChild("vaultSelectorTemplate", { read: TemplateRef }) templateRef: TemplateRef<HTMLElement>;
 
-  private _selectedVault = new BehaviorSubject<string>(null);
+  private _selectedVault = new BehaviorSubject<string | null>(null);
 
   isOpen = false;
   loaded = false;
   organizations$: Observable<Organization[]>;
-  selectedVault$: Observable<string> = this._selectedVault.asObservable();
+  selectedVault$: Observable<string | null> = this._selectedVault.asObservable();
 
   vaultFilter: VaultFilter = new VaultFilter();
   enforcePersonalOwnership = false;

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

@@ -11,7 +11,7 @@ import {
 import { DomSanitizer } from "@angular/platform-browser";
 import { Router } from "@angular/router";
 import { IndividualConfig, ToastrService } from "ngx-toastr";
-import { Subject, takeUntil } from "rxjs";
+import { firstValueFrom, Subject, takeUntil } from "rxjs";
 
 import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
 import { ModalService } from "@bitwarden/angular/services/modal.service";
@@ -175,7 +175,7 @@ export class AppComponent implements OnInit, OnDestroy {
             await this.vaultTimeoutService.lock(message.userId);
             break;
           case "lockAllVaults":
-            for (const userId in this.stateService.accounts.getValue()) {
+            for (const userId in await firstValueFrom(this.stateService.accounts$)) {
               if (userId != null) {
                 await this.vaultTimeoutService.lock(userId);
               }
@@ -429,7 +429,7 @@ export class AppComponent implements OnInit, OnDestroy {
 
   private async updateAppMenu() {
     let updateRequest: MenuUpdateRequest;
-    const stateAccounts = this.stateService.accounts?.getValue();
+    const stateAccounts = await firstValueFrom(this.stateService.accounts$);
     if (stateAccounts == null || Object.keys(stateAccounts).length < 1) {
       updateRequest = {
         accounts: null,
@@ -593,7 +593,8 @@ export class AppComponent implements OnInit, OnDestroy {
   }
 
   private async checkForSystemTimeout(timeout: number): Promise<void> {
-    for (const userId in this.stateService.accounts.getValue()) {
+    const accounts = await firstValueFrom(this.stateService.accounts$);
+    for (const userId in accounts) {
       if (userId == null) {
         continue;
       }

apps/desktop/src/app/guards/login.guard.ts

@@ -1,5 +1,6 @@
 import { Injectable } from "@angular/core";
 import { CanActivate } from "@angular/router";
+import { firstValueFrom } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
@@ -17,7 +18,7 @@ export class LoginGuard implements CanActivate {
   ) {}
 
   async canActivate() {
-    const accounts = this.stateService.accounts.getValue();
+    const accounts = await firstValueFrom(this.stateService.accounts$);
     if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) {
       this.platformUtilsService.showToast("error", null, this.i18nService.t("accountLimitReached"));
       return false;

apps/desktop/src/app/layout/account-switcher.component.ts

@@ -97,7 +97,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
   ) {}
 
   async ngOnInit(): Promise<void> {
-    this.stateService.accounts
+    this.stateService.accounts$
       .pipe(
         concatMap(async (accounts: { [userId: string]: Account }) => {
           for (const userId in accounts) {

apps/desktop/src/app/layout/search/search-bar.service.ts

@@ -8,7 +8,7 @@ export type SearchBarState = {
 
 @Injectable()
 export class SearchBarService {
-  private searchTextSubject = new BehaviorSubject<string>(null);
+  private searchTextSubject = new BehaviorSubject<string | null>(null);
   searchText$ = this.searchTextSubject.asObservable();
 
   private _state = {

apps/desktop/src/services/encryptedMessageHandlerService.ts

@@ -1,3 +1,5 @@
+import { firstValueFrom } from "rxjs";
+
 import { AuthService } from "@bitwarden/common/abstractions/auth.service";
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
 import { MessagingService } from "@bitwarden/common/abstractions/messaging.service";
@@ -78,7 +80,7 @@ export class EncryptedMessageHandlerService {
   }
 
   private async statusCommandHandler(): Promise<AccountStatusResponse[]> {
-    const accounts = this.stateService.accounts.getValue();
+    const accounts = await firstValueFrom(this.stateService.accounts$);
     const activeUserId = await this.stateService.getUserId();
 
     if (!accounts || !Object.keys(accounts)) {

apps/desktop/src/services/nativeMessaging.service.ts

@@ -1,5 +1,6 @@
 import { Injectable } from "@angular/core";
 import { ipcRenderer } from "electron";
+import { firstValueFrom } from "rxjs";
 import Swal from "sweetalert2";
 
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
@@ -58,7 +59,8 @@ export class NativeMessagingService {
       const remotePublicKey = Utils.fromB64ToArray(rawMessage.publicKey).buffer;
 
       // Validate the UserId to ensure we are logged into the same account.
-      const userIds = Object.keys(this.stateService.accounts.getValue());
+      const accounts = await firstValueFrom(this.stateService.accounts$);
+      const userIds = Object.keys(accounts);
       if (!userIds.includes(rawMessage.userId)) {
         ipcRenderer.send("nativeMessagingReply", { command: "wrongUserId", appId: appId });
         return;

apps/web/src/app/vault/vault-filter/shared/vault-filter.service.ts

@@ -18,7 +18,7 @@ import { CollectionView } from "@bitwarden/common/models/view/collection.view";
 
 @Injectable()
 export class VaultFilterService extends BaseVaultFilterService {
-  private _collapsedFilterNodes = new BehaviorSubject<Set<string>>(null);
+  private _collapsedFilterNodes = new BehaviorSubject<Set<string>>(new Set());
   collapsedFilterNodes$: Observable<Set<string>> = this._collapsedFilterNodes.asObservable();
 
   constructor(

libs/common/src/abstractions/state.service.ts

@@ -1,4 +1,4 @@
-import { BehaviorSubject, Observable } from "rxjs";
+import { Observable } from "rxjs";
 
 import { KdfType } from "../enums/kdfType";
 import { ThemeType } from "../enums/themeType";
@@ -27,7 +27,7 @@ import { CollectionView } from "../models/view/collection.view";
 import { SendView } from "../models/view/send.view";
 
 export abstract class StateService<T extends Account = Account> {
-  accounts: BehaviorSubject<{ [userId: string]: T }>;
+  accounts$: Observable<{ [userId: string]: T }>;
   activeAccount$: Observable<string>;
   activeAccountUnlocked$: Observable<boolean>;
 

libs/common/src/services/state.service.ts

@@ -65,8 +65,10 @@ export class StateService<
   TAccount extends Account = Account
 > implements StateServiceAbstraction<TAccount>
 {
-  accounts = new BehaviorSubject<{ [userId: string]: TAccount }>({});
-  private activeAccountSubject = new BehaviorSubject<string>(null);
+  private accountsSubject = new BehaviorSubject<{ [userId: string]: TAccount }>({});
+  accounts$ = this.accountsSubject.asObservable();
+
+  private activeAccountSubject = new BehaviorSubject<string | null>(null);
   activeAccount$ = this.activeAccountSubject.asObservable();
 
   private activeAccountUnlockedSubject = new BehaviorSubject<boolean>(false);
@@ -2530,11 +2532,11 @@ export class StateService<
     await this.pruneInMemoryAccounts();
     await this.state().then((state) => {
       if (state.accounts == null || Object.keys(state.accounts).length < 1) {
-        this.accounts.next(null);
+        this.accountsSubject.next({});
         return;
       }
 
-      this.accounts.next(state.accounts);
+      this.accountsSubject.next(state.accounts);
     });
   }
 

libs/common/src/services/system.service.ts

@@ -1,3 +1,5 @@
+import { firstValueFrom } from "rxjs";
+
 import { AuthService } from "../abstractions/auth.service";
 import { MessagingService } from "../abstractions/messaging.service";
 import { PlatformUtilsService } from "../abstractions/platformUtils.service";
@@ -19,7 +21,7 @@ export class SystemService implements SystemServiceAbstraction {
   ) {}
 
   async startProcessReload(authService: AuthService): Promise<void> {
-    const accounts = this.stateService.accounts.getValue();
+    const accounts = await firstValueFrom(this.stateService.accounts$);
     if (accounts != null) {
       const keys = Object.keys(accounts);
       if (keys.length > 0) {
@@ -56,7 +58,7 @@ export class SystemService implements SystemServiceAbstraction {
   }
 
   private async executeProcessReload() {
-    const accounts = this.stateService.accounts.getValue();
+    const accounts = await firstValueFrom(this.stateService.accounts$);
     const doRefresh =
       accounts == null ||
       Object.keys(accounts).length == 0 ||

libs/common/src/services/vaultTimeout/vaultTimeout.service.ts

@@ -1,3 +1,5 @@
+import { firstValueFrom } from "rxjs";
+
 import { AuthService } from "../../abstractions/auth.service";
 import { CipherService } from "../../abstractions/cipher.service";
 import { CollectionService } from "../../abstractions/collection.service";
@@ -52,7 +54,8 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
       return;
     }
 
-    for (const userId in this.stateService.accounts.getValue()) {
+    const accounts = await firstValueFrom(this.stateService.accounts$);
+    for (const userId in accounts) {
       if (userId != null && (await this.shouldLock(userId))) {
         await this.executeTimeoutAction(userId);
       }