Auth/PM-12077 - Web Process Reload (#11781)

* PM-12077 - Initial work on web process reload - more testing required. * PM-12077 - Clarify comment * PM-12077 - Improving UX of logout with process reload. * PM-12077 - Final tweaks for process reload * PM-12077 - Remove no longer accurate comment. * PM-12077 - Per PR feedback, clean up logout reason --------- Co-authored-by: Matt Bishop <[email protected]>
bitwarden · Nov 20, 2024 · d1499da · d1499da
1 parent 365c7dd
commit d1499da
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 41 deletions.
diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts
@@ -76,7 +76,7 @@ import { DefaultBillingAccountProfileStateService } from "@bitwarden/common/bill
 import { ClientType } from "@bitwarden/common/enums";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service";
-import { ProcessReloadService } from "@bitwarden/common/key-management/services/process-reload.service";
+import { DefaultProcessReloadService } from "@bitwarden/common/key-management/services/default-process-reload.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 { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@@ -1068,7 +1068,7 @@ export default class MainBackground {
       this.taskSchedulerService,
     );
 
-    this.processReloadService = new ProcessReloadService(
+    this.processReloadService = new DefaultProcessReloadService(
       this.pinService,
       this.messagingService,
       systemUtilsServiceReloadCallback,

diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts
@@ -50,7 +50,7 @@ import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/
 import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
 import { ClientType } from "@bitwarden/common/enums";
 import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service";
-import { ProcessReloadService } from "@bitwarden/common/key-management/services/process-reload.service";
+import { DefaultProcessReloadService } from "@bitwarden/common/key-management/services/default-process-reload.service";
 import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto-function.service";
 import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
@@ -221,7 +221,7 @@ const safeProviders: SafeProvider[] = [
   }),
   safeProvider({
     provide: ProcessReloadServiceAbstraction,
-    useClass: ProcessReloadService,
+    useClass: DefaultProcessReloadService,
     deps: [
       PinServiceAbstraction,
       MessagingServiceAbstraction,

diff --git a/apps/web/src/app/app.component.html b/apps/web/src/app/app.component.html
@@ -1 +1,19 @@
-<router-outlet></router-outlet>
+<ng-template #loadingState>
+  <!-- This is the same html from index.html which presents the bitwarden logo and loading spinny properly
+   when the body has the layout_frontend class. Having this match the index allows for a duplicative yet seamless
+   loading state here for process reloading. -->
+  <div class="tw-p-8 tw-flex">
+    <img class="new-logo-themed" alt="Bitwarden" />
+    <div class="spinner-container tw-justify-center">
+      <i
+        class="bwi bwi-spinner bwi-spin bwi-3x tw-text-muted"
+        title="Loading"
+        aria-hidden="true"
+      ></i>
+    </div>
+  </div>
+</ng-template>
+
+<ng-container *ngIf="!loading; else loadingState">
+  <router-outlet></router-outlet>
+</ng-container>
diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts
@@ -6,7 +6,6 @@ import * as jq from "jquery";
 import { Subject, filter, firstValueFrom, map, takeUntil, timeout, catchError, of } from "rxjs";
 
 import { CollectionService } from "@bitwarden/admin-console/common";
-import { LogoutReason } from "@bitwarden/auth/common";
 import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service";
 import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
@@ -17,6 +16,7 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service";
 import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@@ -28,7 +28,7 @@ import { StateEventRunnerService } from "@bitwarden/common/platform/state";
 import { SyncService } from "@bitwarden/common/platform/sync";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
-import { DialogService, ToastOptions, ToastService } from "@bitwarden/components";
+import { DialogService, ToastService } from "@bitwarden/components";
 import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";
 import { KeyService, BiometricStateService } from "@bitwarden/key-management";
 
@@ -60,6 +60,8 @@ export class AppComponent implements OnDestroy, OnInit {
   private isIdle = false;
   private destroy$ = new Subject<void>();
 
+  loading = false;
+
   constructor(
     @Inject(DOCUMENT) private document: Document,
     private broadcasterService: BroadcasterService,
@@ -91,6 +93,7 @@ export class AppComponent implements OnDestroy, OnInit {
     private accountService: AccountService,
     private logService: LogService,
     private sdkService: SdkService,
+    private processReloadService: ProcessReloadServiceAbstraction,
   ) {
     if (flagEnabled("sdk")) {
       // Warn if the SDK for some reason can't be initialized
@@ -161,7 +164,8 @@ export class AppComponent implements OnDestroy, OnInit {
             this.router.navigate(["/"]);
             break;
           case "logout":
-            await this.logOut(message.logoutReason, message.redirect);
+            // note: the message.logoutReason isn't consumed anymore because of the process reload clearing any toasts.
+            await this.logOut(message.redirect);
             break;
           case "lockVault":
             await this.vaultTimeoutService.lock();
@@ -170,9 +174,8 @@ export class AppComponent implements OnDestroy, OnInit {
             // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
             // eslint-disable-next-line @typescript-eslint/no-floating-promises
             this.notificationsService.updateConnection(false);
-            // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-            // eslint-disable-next-line @typescript-eslint/no-floating-promises
-            this.router.navigate(["lock"]);
+
+            await this.processReloadService.startProcessReload(this.authService);
             break;
           case "lockedUrl":
             break;
@@ -272,33 +275,16 @@ export class AppComponent implements OnDestroy, OnInit {
     this.destroy$.complete();
   }
 
-  private async displayLogoutReason(logoutReason: LogoutReason) {
-    let toastOptions: ToastOptions;
-    switch (logoutReason) {
-      case "invalidSecurityStamp":
-      case "sessionExpired": {
-        toastOptions = {
-          variant: "warning",
-          title: this.i18nService.t("loggedOut"),
-          message: this.i18nService.t("loginExpired"),
-        };
-        break;
-      }
-      default: {
-        toastOptions = {
-          variant: "info",
-          title: this.i18nService.t("loggedOut"),
-          message: this.i18nService.t("loggedOutDesc"),
-        };
-        break;
-      }
-    }
-
-    this.toastService.showToast(toastOptions);
-  }
+  private async logOut(redirect = true) {
+    // Ensure the loading state is applied before proceeding to avoid a flash
+    // of the login screen before the process reload fires.
+    this.ngZone.run(() => {
+      this.loading = true;
+      document.body.classList.add("layout_frontend");
+    });
 
-  private async logOut(logoutReason: LogoutReason, redirect = true) {
-    await this.displayLogoutReason(logoutReason);
+    // Note: we don't display a toast logout reason anymore as the process reload
+    // will prevent any toasts from being displayed long enough to be read
 
     await this.eventUploadService.uploadEvents();
     const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id)));
@@ -334,10 +320,14 @@ export class AppComponent implements OnDestroy, OnInit {
       await logoutPromise;
 
       if (redirect) {
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.router.navigate(["/"]);
+        await this.router.navigate(["/"]);
       }
+
+      await this.processReloadService.startProcessReload(this.authService);
+
+      // Normally we would need to reset the loading state to false or remove the layout_frontend
+      // class from the body here, but the process reload completely reloads the app so
+      // it handles it.
     }, userId);
   }
 

diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts
@@ -49,6 +49,7 @@ import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config
 import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
 import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
 import { ClientType } from "@bitwarden/common/enums";
+import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service";
 import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
@@ -95,6 +96,7 @@ import {
 import { AcceptOrganizationInviteService } from "../auth/organization-invite/accept-organization.service";
 import { HtmlStorageService } from "../core/html-storage.service";
 import { I18nService } from "../core/i18n.service";
+import { WebProcessReloadService } from "../key-management/services/web-process-reload.service";
 import { WebBiometricsService } from "../key-management/web-biometric.service";
 import { WebEnvironmentService } from "../platform/web-environment.service";
 import { WebMigrationRunner } from "../platform/web-migration-runner";
@@ -281,6 +283,11 @@ const safeProviders: SafeProvider[] = [
     useClass: flagEnabled("sdk") ? WebSdkClientFactory : NoopSdkClientFactory,
     deps: [],
   }),
+  safeProvider({
+    provide: ProcessReloadServiceAbstraction,
+    useClass: WebProcessReloadService,
+    deps: [WINDOW],
+  }),
   safeProvider({
     provide: LoginEmailService,
     useClass: LoginEmailService,

diff --git a/apps/web/src/app/key-management/services/web-process-reload.service.ts b/apps/web/src/app/key-management/services/web-process-reload.service.ts
@@ -0,0 +1,14 @@
+import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
+import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service";
+
+export class WebProcessReloadService implements ProcessReloadServiceAbstraction {
+  constructor(private window: Window) {}
+
+  async startProcessReload(authService: AuthService): Promise<void> {
+    this.window.location.reload();
+  }
+
+  cancelProcessReload(): void {
+    return;
+  }
+}
diff --git a/apps/web/src/index.html b/apps/web/src/index.html
@@ -15,6 +15,7 @@
   </head>
   <body class="layout_frontend">
     <app-root>
+      <!-- Note: any changes to this html need to be made in the app.component.html as well -->
       <div class="tw-p-8 tw-flex">
         <img class="new-logo-themed" alt="Bitwarden" />
         <div class="spinner-container tw-justify-center">

diff --git a/libs/angular/src/auth/guards/lock.guard.ts b/libs/angular/src/auth/guards/lock.guard.ts
@@ -42,6 +42,12 @@ export function lockGuard(): CanActivateFn {
 
     const activeUser = await firstValueFrom(accountService.activeAccount$);
 
+    // If no active user, redirect to root:
+    // scenario context: user logs out on lock screen and app will reload lock comp without active user
+    if (!activeUser) {
+      return router.createUrlTree(["/"]);
+    }
+
     const authStatus = await firstValueFrom(authService.authStatusFor$(activeUser.id));
     if (authStatus !== AuthenticationStatus.Locked) {
       return router.createUrlTree(["/"]);

diff --git a/...gement/services/process-reload.service.ts → ...ervices/default-process-reload.service.ts b/...gement/services/process-reload.service.ts → ...ervices/default-process-reload.service.ts
@@ -12,7 +12,7 @@ import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum";
 import { UserId } from "../../types/guid";
 import { ProcessReloadServiceAbstraction } from "../abstractions/process-reload.service";
 
-export class ProcessReloadService implements ProcessReloadServiceAbstraction {
+export class DefaultProcessReloadService implements ProcessReloadServiceAbstraction {
   private reloadInterval: any = null;
 
   constructor(