From d1499da7932cbc4ebc1407d0ca3ed435003320d5 Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:54:27 -0500 Subject: [PATCH] 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 --- .../browser/src/background/main.background.ts | 4 +- .../src/app/services/services.module.ts | 4 +- apps/web/src/app/app.component.html | 20 ++++++- apps/web/src/app/app.component.ts | 60 ++++++++----------- apps/web/src/app/core/core.module.ts | 7 +++ .../services/web-process-reload.service.ts | 14 +++++ apps/web/src/index.html | 1 + libs/angular/src/auth/guards/lock.guard.ts | 6 ++ ...e.ts => default-process-reload.service.ts} | 2 +- 9 files changed, 77 insertions(+), 41 deletions(-) create mode 100644 apps/web/src/app/key-management/services/web-process-reload.service.ts rename libs/common/src/key-management/services/{process-reload.service.ts => default-process-reload.service.ts} (97%) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fdfd5740ba..a74f187c76 100644 --- 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 index a2195fbd5a..d07feabd3b 100644 --- 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 index 0680b43f9c..a39c045e0e 100644 --- a/apps/web/src/app/app.component.html +++ b/apps/web/src/app/app.component.html @@ -1 +1,19 @@ - + + +
+ Bitwarden + +
+ + + + + diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 704960c14c..468ae88e0c 100644 --- 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(); + 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; - } - } + 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"); + }); - this.toastService.showToast(toastOptions); - } - - 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 index d1fa3f8ba8..2209a10d58 100644 --- 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 new file mode 100644 index 0000000000..c542c97c0e --- /dev/null +++ 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 { + this.window.location.reload(); + } + + cancelProcessReload(): void { + return; + } +} diff --git a/apps/web/src/index.html b/apps/web/src/index.html index ce1a955b88..0b8ea86491 100644 --- a/apps/web/src/index.html +++ b/apps/web/src/index.html @@ -15,6 +15,7 @@ +
Bitwarden