From 87b6651f8e71a3be65eeda2a9160a2bde286196d Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 18 Dec 2023 14:23:43 -0500 Subject: [PATCH] [pm-5287] fix account switch logout routing (#7231) * Navigate to home from account switcher Also updates the main background handling of logout to either finish switch or logout, depending on which occurred * Prefer observable guards we were racing the account switch process on `accountService` and this async guard. It only depended on account status, which is available from `accountService`, so the correct move was to observe that status. The unauthGuardFn allows for updating homepage depending on window state because popout windows have different nav to other locations. --- .../account-switcher.component.ts | 2 +- .../browser/src/background/main.background.ts | 9 +++-- apps/browser/src/popup/app-routing.module.ts | 25 +++++++----- libs/angular/src/auth/guards/unauth.guard.ts | 40 ++++++++++++++++++- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts index d7661fa9d44..3b75f09c3c2 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts @@ -103,7 +103,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { this.messagingService.send("logout"); } - this.router.navigate(["account-switcher"]); + this.router.navigate(["home"]); } ngOnDestroy() { diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 7e97797fc5b..6994ac6c6ff 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -906,14 +906,15 @@ export default class MainBackground { const currentUserId = await this.stateService.getUserId(); const newActiveUser = await this.stateService.clean({ userId: userId }); + if (userId == null || userId === currentUserId) { + this.searchService.clearIndex(); + } + if (newActiveUser != null) { // we have a new active user, do not continue tearing down application await this.switchAccount(newActiveUser as UserId); this.messagingService.send("switchAccountFinish"); - } - - if (userId == null || userId === currentUserId) { - this.searchService.clearIndex(); + } else { this.messagingService.send("doneLoggingOut", { expired: expired, userId: userId }); } diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index f012dbe9c99..bbfdee98b9f 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -6,7 +6,7 @@ import { AuthGuard, lockGuard, tdeDecryptionRequiredGuard, - UnauthGuard, + unauthGuardFn, } from "@bitwarden/angular/auth/guards"; import { canAccessFeature } from "@bitwarden/angular/platform/guard/feature-flag.guard"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -28,6 +28,7 @@ import { TwoFactorOptionsComponent } from "../auth/popup/two-factor-options.comp import { TwoFactorComponent } from "../auth/popup/two-factor.component"; import { UpdateTempPasswordComponent } from "../auth/popup/update-temp-password.component"; import { AutofillComponent } from "../autofill/popup/settings/autofill.component"; +import BrowserPopupUtils from "../platform/popup/browser-popup-utils"; import { GeneratorComponent } from "../tools/popup/generator/generator.component"; import { PasswordGeneratorHistoryComponent } from "../tools/popup/generator/password-generator-history.component"; import { SendAddEditComponent } from "../tools/popup/send/send-add-edit.component"; @@ -57,6 +58,12 @@ import { SettingsComponent } from "./settings/settings.component"; import { SyncComponent } from "./settings/sync.component"; import { TabsComponent } from "./tabs.component"; +const unauthRouteOverrides = { + homepage: () => { + return BrowserPopupUtils.inPopout(window) ? "/tabs/vault" : "/tabs/current"; + }, +}; + const routes: Routes = [ { path: "", @@ -74,7 +81,7 @@ const routes: Routes = [ { path: "home", component: HomeComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "home" }, }, { @@ -86,7 +93,7 @@ const routes: Routes = [ { path: "login", component: LoginComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "login" }, }, { @@ -110,13 +117,13 @@ const routes: Routes = [ { path: "2fa", component: TwoFactorComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "2fa" }, }, { path: "2fa-options", component: TwoFactorOptionsComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "2fa-options" }, }, { @@ -130,7 +137,7 @@ const routes: Routes = [ { path: "sso", component: SsoComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "sso" }, }, { @@ -147,19 +154,19 @@ const routes: Routes = [ { path: "register", component: RegisterComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "register" }, }, { path: "hint", component: HintComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "hint" }, }, { path: "environment", component: EnvironmentComponent, - canActivate: [UnauthGuard], + canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { state: "environment" }, }, { diff --git a/libs/angular/src/auth/guards/unauth.guard.ts b/libs/angular/src/auth/guards/unauth.guard.ts index e6597dcabca..35c59b57449 100644 --- a/libs/angular/src/auth/guards/unauth.guard.ts +++ b/libs/angular/src/auth/guards/unauth.guard.ts @@ -1,9 +1,14 @@ -import { Injectable } from "@angular/core"; -import { CanActivate, Router } from "@angular/router"; +import { Injectable, inject } from "@angular/core"; +import { CanActivate, CanActivateFn, Router, UrlTree } from "@angular/router"; +import { Observable, map } from "rxjs"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +/** + * @deprecated use unauthGuardFn function instead + */ @Injectable() export class UnauthGuard implements CanActivate { protected homepage = "vault"; @@ -26,3 +31,34 @@ export class UnauthGuard implements CanActivate { return this.router.createUrlTree([this.homepage]); } } + +type UnauthRoutes = { + homepage: () => string; + locked: string; +}; + +const defaultRoutes: UnauthRoutes = { + homepage: () => "/vault", + locked: "/lock", +}; + +function unauthGuard(routes: UnauthRoutes): Observable { + const accountService = inject(AccountService); + const router = inject(Router); + + return accountService.activeAccount$.pipe( + map((accountData) => { + if (accountData == null || accountData.status === AuthenticationStatus.LoggedOut) { + return true; + } else if (accountData.status === AuthenticationStatus.Locked) { + return router.createUrlTree([routes.locked]); + } else { + return router.createUrlTree([routes.homepage()]); + } + }), + ); +} + +export function unauthGuardFn(overrides: Partial = {}): CanActivateFn { + return () => unauthGuard({ ...defaultRoutes, ...overrides }); +}