From ac899bebeb33374e5b156e0f2250d71f8e57b3bb Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Wed, 6 Dec 2023 09:22:48 -0800 Subject: [PATCH] [PM-194] Browser Account Switcher UI (#6772) * Handle switch messaging TODO: handle loading state for account switcher * Async updates required for state * Fallback to email for current account avatar * Await un-awaited promises * Remove unnecessary Prune Prune was getting confused in browser and deleting memory in browser on account switch. This method isn't needed since logout already removes memory data, which is the condition for pruning * Fix temp password in browser * Use direct memory access until data is serializable Safari uses a different message object extraction than firefox/chrome and is removing `UInt8Array`s. Until all data passed into StorageService is guaranteed serializable, we need to use direct access in state service * Reload badge and context menu on switch * Gracefully switch account as they log out. * Maintain location on account switch * Remove unused state definitions * Prefer null for state undefined can be misinterpreted to indicate a value has not been set. * Hack: structured clone in memory storage We are currently getting dead objects on account switch due to updating the object in the foreground state service. However, the storage service is owned by the background. This structured clone hack ensures that all objects stored in memory are owned by the appropriate context * Null check nullable values active account can be null, so we should include null safety in the equality * Correct background->foreground switch command * Already providing background memory storage * Handle connection and clipboard on switch account * Prefer strict equal * Ensure structuredClone is available to jsdom This is a deficiency in jsdom -- https://github.com/jsdom/jsdom/issues/3363 -- structured clone is well supported. * Fixup types in faker class * add avatar and simple navigation to header * add options buttons * add app-header to necessary pages * add back button and adjust avatar sizes * add helper text when account limit reached * convert magic number to constant * add clarifying comment * adjust homepage header styles * navigate to previousp page upon avatar click when already on '/account-switcher' * move account UI to own component * add i18n * show correct auth status * add aria-hidden to icons * use listbox role * add screen reader accessibility to account component * more SR a11y updates to account component * add hover and focus states to avatar * refactor hover and focus states for avatar * add screen reader text for avatar * add slide-down animation on account switcher close * remove comment * setup account component story * add all stories * move navigation call to account component * implement account lock * add button hover effect * implement account logout * implement lockAll accounts functionality * replace 'any' with custom type * add account switcher button to /home login page * use
tag (enables scrolling) * change temp server filler name * temporarily remove server arg from account story * don't show avatar on /home if no accounts, and don't show 'lock'/'logout' buttons if no currentAccount * add translation and a11y to /home avatar * add correct server url to account component * add 'server' to AccountOption type * Enabled account switching client-side feature flag. * add slide-in transition to /account-switcher page * change capitalization to reflect figma design * make screen reader read active account email, address more capitalization * fix web avatar misalignment * make avatar color based on user settings and in sync with other clients * make property private * change accountOptions to availableAccounts for clarity * refactor to remove 'else' template ref * remove empty scss rule * use tailwind instead of scss * rename isSelected to isActive * add 'isButton' to /home page avatar * move files to services folder * update import * Remove duplicate active account button * Move no account button to current-account component * Always complete logging out Fixes PM-4866 * make screenreader read off email, not name * refactor avatar for button case * Do not next object updates StateService's init was calling `updateState` at multiple layers, once overall and then again for each account updated. Because we were not maintaining a single state object through the process, it was ending up in a consistent, but incomplete state. Fixed by returning the updated state everywhere. This very well may not be all the bugs associated with this * Treat null switch account as no active user * Listen for switchAccountFinish before routing (#6950) * adjust avatar style when wrapped in a button * show alt text for favicon setting * move stories to browser * Send Finish Message on null * Dynamically set active user when locking all This is required because some user lock states are not recoverable after process reload (those with logout timeout). This waits until reload is occurring, then sets the next user appropriately * Move Finished Message to Finally Block Fix tests * Drop problematic key migration Fixes PM-4933. This was an instance of foreground/background collision when writing state. We have several other fallbacks of clearing these deprecated keys. * Prefer location to homebrew router service * Initialize account disk cache from background Uses the `isRecoveredSession` bool to re-initialize foreground caches from a background message. This avoids a lengthy first-read for foregrounds * PM-4865 - Browser Acct Switcher - only show lock btn for lockable accounts (#6991) * Lock of active account first, when locking multiple. Fixes PM-4996 * Fix linter * Hide lock now for locked users (#7020) * Hide lock now for locked users * Prefer disabling button to removing * Add tooltip to TDE without unlock method * Load all auth states on state init (#7027) This is a temporary fix until the owning services can update state themselves. It uses the presence of an auto key to surmise unlocked state on init. This is safe since it's run only once on extension start. * Ps/pm 5004/add load to account switcher (#7032) * Add load spinner to account switcher * Remove ul list icons * Properly size account switcher in popout * [PM-5005] Prevent Double Navigation (#7035) * Delete Overriden Method * Add Lock Transition * truncate email and server name * remove account.stories.ts (will add in separate PR) * Do not switch user at reload if no user is active * fix prettier issues --------- Co-authored-by: Matt Gibson Co-authored-by: Todd Martin Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Co-authored-by: bnagawiecki <107435978+bnagawiecki@users.noreply.github.com> --- apps/browser/config/development.json | 3 +- apps/browser/config/production.json | 3 +- apps/browser/src/_locales/en/messages.json | 39 +++++ .../account-switcher.component.html | 75 +++++++++- .../account-switcher.component.ts | 108 ++++++++++++-- .../account-switching/account.component.html | 49 ++++++ .../account-switching/account.component.ts | 55 +++++++ .../current-account.component.html | 38 ++++- .../current-account.component.ts | 27 ++-- .../services/account-switcher.service.spec.ts | 57 +++++-- .../services/account-switcher.service.ts | 139 ++++++++++++++++++ .../services/current-account.service.ts | 44 ++++++ .../src/auth/popup/home.component.html | 2 +- apps/browser/src/auth/popup/home.component.ts | 7 + .../services/account-switcher.service.ts | 60 -------- .../browser/src/background/main.background.ts | 49 +++--- .../src/platform/browser/browser-api.ts | 5 + .../services/browser-state.service.ts | 22 +++ .../src/popup/app-routing.animations.ts | 5 + apps/browser/src/popup/app.component.ts | 4 +- apps/browser/src/popup/app.module.ts | 2 + apps/browser/src/popup/scss/base.scss | 9 +- .../src/popup/settings/options.component.html | 4 +- .../popup/settings/settings.component.html | 4 +- .../popup/generator/generator.component.html | 4 +- .../popup/send/send-groupings.component.html | 6 +- .../vault/current-tab.component.html | 6 +- .../src/app/services/services.module.ts | 2 + .../src/auth/components/set-pin.component.ts | 2 - .../platform/abstractions/state.service.ts | 1 + .../src/platform/services/state.service.ts | 55 ++++--- .../src/platform/services/system.service.ts | 18 ++- libs/components/tailwind.config.js | 1 + 33 files changed, 739 insertions(+), 166 deletions(-) create mode 100644 apps/browser/src/auth/popup/account-switching/account.component.html create mode 100644 apps/browser/src/auth/popup/account-switching/account.component.ts rename apps/browser/src/auth/popup/{ => account-switching}/services/account-switcher.service.spec.ts (62%) create mode 100644 apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts create mode 100644 apps/browser/src/auth/popup/account-switching/services/current-account.service.ts delete mode 100644 apps/browser/src/auth/popup/services/account-switcher.service.ts diff --git a/apps/browser/config/development.json b/apps/browser/config/development.json index eafd0ffd878..1b628c173ce 100644 --- a/apps/browser/config/development.json +++ b/apps/browser/config/development.json @@ -7,6 +7,7 @@ }, "flags": { "showPasswordless": true, - "enableCipherKeyEncryption": false + "enableCipherKeyEncryption": false, + "accountSwitching": true } } diff --git a/apps/browser/config/production.json b/apps/browser/config/production.json index f57c3d9bc38..027003f6c75 100644 --- a/apps/browser/config/production.json +++ b/apps/browser/config/production.json @@ -1,5 +1,6 @@ { "flags": { - "enableCipherKeyEncryption": false + "enableCipherKeyEncryption": false, + "accountSwitching": true } } diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index b07f9307255..361d8076ba2 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -365,6 +365,9 @@ "unlockMethodNeededToChangeTimeoutActionDesc": { "message": "Set up an unlock method to change your vault timeout action." }, + "unlockMethodNeeded": { + "message": "Set up an unlock method in Settings" + }, "rateExtension": { "message": "Rate the extension" }, @@ -405,6 +408,9 @@ "lockNow": { "message": "Lock now" }, + "lockAll": { + "message": "Lock all" + }, "immediately": { "message": "Immediately" }, @@ -1131,6 +1137,9 @@ "faviconDesc": { "message": "Show a recognizable image next to each login." }, + "faviconDescAlt": { + "message": "Show a recognizable image next to each login. Applies to all logged in accounts." + }, "enableBadgeCounter": { "message": "Show badge counter" }, @@ -2796,5 +2805,35 @@ }, "lastPassYubikeyDesc": { "message": "Insert the YubiKey associated with your LastPass account into your computer's USB port, then touch its button." + }, + "switchAccount": { + "message": "Switch account" + }, + "switchAccounts": { + "message": "Switch accounts" + }, + "switchToAccount": { + "message": "Switch to account" + }, + "activeAccount": { + "message": "Active account" + }, + "accountLimitReached": { + "message": "Account limit reached. Log out of an account to add another." + }, + "active": { + "message": "active" + }, + "locked": { + "message": "locked" + }, + "unlocked": { + "message": "unlocked" + }, + "server": { + "message": "server" + }, + "hostedAt": { + "message": "hosted at" } } diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html index bde9c3f6a02..f7421870143 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html @@ -1,7 +1,70 @@ -
-
- + +
+
-
+
{{ "switchAccounts" | i18n }}
+ + +
+ +
+
+
+
+
    +
  • + +
    • +
+ +

+ {{ "accountLimitReached" | i18n }} +

+
+ +
+
{{ "options" | i18n }}
+
+ + + +
+
+
+
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 8b6b660cd70..d7661fa9d44 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 @@ -1,25 +1,113 @@ -import { Component } from "@angular/core"; +import { Location } from "@angular/common"; +import { Component, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; +import { Subject, firstValueFrom, map, switchMap, takeUntil } from "rxjs"; -import { BrowserRouterService } from "../../../platform/popup/services/browser-router.service"; -import { AccountSwitcherService } from "../services/account-switcher.service"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; +import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { DialogService } from "@bitwarden/components"; + +import { AccountSwitcherService } from "./services/account-switcher.service"; @Component({ templateUrl: "account-switcher.component.html", }) -export class AccountSwitcherComponent { +export class AccountSwitcherComponent implements OnInit, OnDestroy { + readonly lockedStatus = AuthenticationStatus.Locked; + private destroy$ = new Subject(); + + loading = false; + activeUserCanLock = false; + constructor( private accountSwitcherService: AccountSwitcherService, + private accountService: AccountService, + private vaultTimeoutService: VaultTimeoutService, + private messagingService: MessagingService, + private dialogService: DialogService, + private location: Location, private router: Router, - private routerService: BrowserRouterService, + private vaultTimeoutSettingsService: VaultTimeoutSettingsService, ) {} - get accountOptions$() { - return this.accountSwitcherService.accountOptions$; + get accountLimit() { + return this.accountSwitcherService.ACCOUNT_LIMIT; } - async selectAccount(id: string) { - await this.accountSwitcherService.selectAccount(id); - this.router.navigate([this.routerService.getPreviousUrl() ?? "/home"]); + get specialAddAccountId() { + return this.accountSwitcherService.SPECIAL_ADD_ACCOUNT_ID; + } + + get availableAccounts$() { + return this.accountSwitcherService.availableAccounts$; + } + + get currentAccount$() { + return this.accountService.activeAccount$; + } + + async ngOnInit() { + const availableVaultTimeoutActions = await firstValueFrom( + this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + this.activeUserCanLock = availableVaultTimeoutActions.includes(VaultTimeoutAction.Lock); + } + + back() { + this.location.back(); + } + + async lock(userId?: string) { + this.loading = true; + await this.vaultTimeoutService.lock(userId ? userId : null); + this.router.navigate(["lock"]); + } + + async lockAll() { + this.loading = true; + this.availableAccounts$ + .pipe( + map((accounts) => + accounts + .filter((account) => account.id !== this.specialAddAccountId) + .sort((a, b) => (a.isActive ? -1 : b.isActive ? 1 : 0)) // Log out of the active account first + .map((account) => account.id), + ), + switchMap(async (accountIds) => { + if (accountIds.length === 0) { + return; + } + + // Must lock active (first) account first, then order doesn't matter + await this.vaultTimeoutService.lock(accountIds.shift()); + await Promise.all(accountIds.map((id) => this.vaultTimeoutService.lock(id))); + }), + takeUntil(this.destroy$), + ) + .subscribe(() => this.router.navigate(["lock"])); + } + + async logOut() { + this.loading = true; + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "logOut" }, + content: { key: "logOutConfirmation" }, + type: "info", + }); + + if (confirmed) { + this.messagingService.send("logout"); + } + + this.router.navigate(["account-switcher"]); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); } } diff --git a/apps/browser/src/auth/popup/account-switching/account.component.html b/apps/browser/src/auth/popup/account-switching/account.component.html new file mode 100644 index 00000000000..52edeabc7b5 --- /dev/null +++ b/apps/browser/src/auth/popup/account-switching/account.component.html @@ -0,0 +1,49 @@ + + + diff --git a/apps/browser/src/auth/popup/account-switching/account.component.ts b/apps/browser/src/auth/popup/account-switching/account.component.ts new file mode 100644 index 00000000000..2455aee1b86 --- /dev/null +++ b/apps/browser/src/auth/popup/account-switching/account.component.ts @@ -0,0 +1,55 @@ +import { CommonModule, Location } from "@angular/common"; +import { Component, EventEmitter, Input, Output } from "@angular/core"; +import { Router } from "@angular/router"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { AvatarModule } from "@bitwarden/components"; + +import { AccountSwitcherService, AvailableAccount } from "./services/account-switcher.service"; + +@Component({ + standalone: true, + selector: "auth-account", + templateUrl: "account.component.html", + imports: [CommonModule, JslibModule, AvatarModule], +}) +export class AccountComponent { + @Input() account: AvailableAccount; + @Output() loading = new EventEmitter(); + + constructor( + private accountSwitcherService: AccountSwitcherService, + private router: Router, + private location: Location, + private i18nService: I18nService, + ) {} + + get specialAccountAddId() { + return this.accountSwitcherService.SPECIAL_ADD_ACCOUNT_ID; + } + + async selectAccount(id: string) { + this.loading.emit(true); + await this.accountSwitcherService.selectAccount(id); + + if (id === this.specialAccountAddId) { + this.router.navigate(["home"]); + } else { + this.location.back(); + } + } + + get status() { + if (this.account.isActive && this.account.status !== AuthenticationStatus.Locked) { + return { text: this.i18nService.t("active"), icon: "bwi-check-circle" }; + } + + if (this.account.status === AuthenticationStatus.Unlocked) { + return { text: this.i18nService.t("unlocked"), icon: "bwi-unlock" }; + } + + return { text: this.i18nService.t("locked"), icon: "bwi-lock" }; + } +} diff --git a/apps/browser/src/auth/popup/account-switching/current-account.component.html b/apps/browser/src/auth/popup/account-switching/current-account.component.html index 189ea4c736f..777d3af3047 100644 --- a/apps/browser/src/auth/popup/account-switching/current-account.component.html +++ b/apps/browser/src/auth/popup/account-switching/current-account.component.html @@ -1,5 +1,35 @@ -
-
- -
+
+ + + +
diff --git a/apps/browser/src/auth/popup/account-switching/current-account.component.ts b/apps/browser/src/auth/popup/account-switching/current-account.component.ts index 480a3a6a10a..df6cb322507 100644 --- a/apps/browser/src/auth/popup/account-switching/current-account.component.ts +++ b/apps/browser/src/auth/popup/account-switching/current-account.component.ts @@ -1,9 +1,8 @@ +import { Location } from "@angular/common"; import { Component } from "@angular/core"; -import { Router } from "@angular/router"; -import { map } from "rxjs"; +import { ActivatedRoute, Router } from "@angular/router"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { CurrentAccountService } from "./services/current-account.service"; @Component({ selector: "app-current-account", @@ -11,23 +10,21 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; }) export class CurrentAccountComponent { constructor( - private accountService: AccountService, + private currentAccountService: CurrentAccountService, private router: Router, + private location: Location, + private route: ActivatedRoute, ) {} get currentAccount$() { - return this.accountService.activeAccount$; - } - - get currentAccountName$() { - return this.currentAccount$.pipe( - map((a) => { - return Utils.isNullOrWhitespace(a.name) ? a.email : a.name; - }), - ); + return this.currentAccountService.currentAccount$; } async currentAccountClicked() { - await this.router.navigate(["/account-switcher"]); + if (this.route.snapshot.data.state.includes("account-switcher")) { + this.location.back(); + } else { + this.router.navigate(["/account-switcher"]); + } } } diff --git a/apps/browser/src/auth/popup/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts similarity index 62% rename from apps/browser/src/auth/popup/services/account-switcher.service.spec.ts rename to apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index cdd88360b73..9845fac1dad 100644 --- a/apps/browser/src/auth/popup/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -3,6 +3,8 @@ import { BehaviorSubject, firstValueFrom, timeout } from "rxjs"; import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { UserId } from "@bitwarden/common/types/guid"; @@ -16,6 +18,8 @@ describe("AccountSwitcherService", () => { const accountService = mock(); const stateService = mock(); const messagingService = mock(); + const environmentService = mock(); + const logService = mock(); let accountSwitcherService: AccountSwitcherService; @@ -27,10 +31,12 @@ describe("AccountSwitcherService", () => { accountService, stateService, messagingService, + environmentService, + logService, ); }); - describe("accountOptions$", () => { + describe("availableAccounts$", () => { it("should return all accounts and an add account option when accounts are less than 5", async () => { const user1AccountInfo: AccountInfo = { name: "Test User 1", @@ -45,14 +51,14 @@ describe("AccountSwitcherService", () => { activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "1" as UserId })); const accounts = await firstValueFrom( - accountSwitcherService.accountOptions$.pipe(timeout(20)), + accountSwitcherService.availableAccounts$.pipe(timeout(20)), ); expect(accounts).toHaveLength(2); expect(accounts[0].id).toBe("1"); - expect(accounts[0].isSelected).toBeTruthy(); + expect(accounts[0].isActive).toBeTruthy(); expect(accounts[1].id).toBe("addAccount"); - expect(accounts[1].isSelected).toBeFalsy(); + expect(accounts[1].isActive).toBeFalsy(); }); it.each([5, 6])( @@ -71,7 +77,7 @@ describe("AccountSwitcherService", () => { Object.assign(seedAccounts["1" as UserId], { id: "1" as UserId }), ); - const accounts = await firstValueFrom(accountSwitcherService.accountOptions$); + const accounts = await firstValueFrom(accountSwitcherService.availableAccounts$); expect(accounts).toHaveLength(numberOfAccounts); accounts.forEach((account) => { @@ -83,16 +89,46 @@ describe("AccountSwitcherService", () => { describe("selectAccount", () => { it("initiates an add account logic when add account is selected", async () => { - await accountSwitcherService.selectAccount("addAccount"); + let listener: ( + message: { command: string; userId: string }, + sender: unknown, + sendResponse: unknown, + ) => void = null; + jest.spyOn(chrome.runtime.onMessage, "addListener").mockImplementation((addedListener) => { + listener = addedListener; + }); - expect(stateService.setActiveUser).toBeCalledWith(null); - expect(stateService.setRememberedEmail).toBeCalledWith(null); + const removeListenerSpy = jest.spyOn(chrome.runtime.onMessage, "removeListener"); - expect(accountService.switchAccount).not.toBeCalled(); + const selectAccountPromise = accountSwitcherService.selectAccount("addAccount"); + + expect(listener).not.toBeNull(); + listener({ command: "switchAccountFinish", userId: null }, undefined, undefined); + + await selectAccountPromise; + + expect(accountService.switchAccount).toBeCalledWith(null); + + expect(removeListenerSpy).toBeCalledTimes(1); }); it("initiates an account switch with an account id", async () => { - await accountSwitcherService.selectAccount("1"); + let listener: ( + message: { command: string; userId: string }, + sender: unknown, + sendResponse: unknown, + ) => void; + jest.spyOn(chrome.runtime.onMessage, "addListener").mockImplementation((addedListener) => { + listener = addedListener; + }); + + const removeListenerSpy = jest.spyOn(chrome.runtime.onMessage, "removeListener"); + + const selectAccountPromise = accountSwitcherService.selectAccount("1"); + + listener({ command: "switchAccountFinish", userId: "1" }, undefined, undefined); + + await selectAccountPromise; expect(accountService.switchAccount).toBeCalledWith("1"); expect(messagingService.send).toBeCalledWith( @@ -101,6 +137,7 @@ describe("AccountSwitcherService", () => { return payload.userId === "1"; }), ); + expect(removeListenerSpy).toBeCalledTimes(1); }); }); }); diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts new file mode 100644 index 00000000000..19ede98e2c3 --- /dev/null +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts @@ -0,0 +1,139 @@ +import { Injectable } from "@angular/core"; +import { + Observable, + combineLatest, + filter, + firstValueFrom, + map, + switchMap, + throwError, + timeout, +} from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { fromChromeEvent } from "../../../../platform/browser/from-chrome-event"; + +export type AvailableAccount = { + name: string; + email?: string; + id: string; + isActive: boolean; + server?: string; + status?: AuthenticationStatus; + avatarColor?: string; +}; + +@Injectable({ + providedIn: "root", +}) +export class AccountSwitcherService { + static incompleteAccountSwitchError = "Account switch did not complete."; + + ACCOUNT_LIMIT = 5; + SPECIAL_ADD_ACCOUNT_ID = "addAccount"; + availableAccounts$: Observable; + + switchAccountFinished$: Observable; + + constructor( + private accountService: AccountService, + private stateService: StateService, + private messagingService: MessagingService, + private environmentService: EnvironmentService, + private logService: LogService, + ) { + this.availableAccounts$ = combineLatest([ + this.accountService.accounts$, + this.accountService.activeAccount$, + ]).pipe( + switchMap(async ([accounts, activeAccount]) => { + const accountEntries = Object.entries(accounts).filter( + ([_, account]) => account.status !== AuthenticationStatus.LoggedOut, + ); + // Accounts shouldn't ever be more than ACCOUNT_LIMIT but just in case do a greater than + const hasMaxAccounts = accountEntries.length >= this.ACCOUNT_LIMIT; + const options: AvailableAccount[] = await Promise.all( + accountEntries.map(async ([id, account]) => { + return { + name: account.name ?? account.email, + email: account.email, + id: id, + server: await this.environmentService.getHost(id), + status: account.status, + isActive: id === activeAccount?.id, + avatarColor: await this.stateService.getAvatarColor({ userId: id }), + }; + }), + ); + + if (!hasMaxAccounts) { + options.push({ + name: "Add account", + id: this.SPECIAL_ADD_ACCOUNT_ID, + isActive: activeAccount?.id == null, + }); + } + + return options; + }), + ); + + // Create a reusable observable that listens to the the switchAccountFinish message and returns the userId from the message + this.switchAccountFinished$ = fromChromeEvent<[message: { command: string; userId: string }]>( + chrome.runtime.onMessage, + ).pipe( + filter(([message]) => message.command === "switchAccountFinish"), + map(([message]) => message.userId), + ); + } + + get specialAccountAddId() { + return this.SPECIAL_ADD_ACCOUNT_ID; + } + + async selectAccount(id: string) { + if (id === this.SPECIAL_ADD_ACCOUNT_ID) { + id = null; + } + + // Creates a subscription to the switchAccountFinished observable but further + // filters it to only care about the current userId. + const switchAccountFinishedPromise = firstValueFrom( + this.switchAccountFinished$.pipe( + filter((userId) => userId === id), + timeout({ + // Much longer than account switching is expected to take for normal accounts + // but the account switching process includes a possible full sync so we need to account + // for very large accounts and want to still have a timeout + // to avoid a promise that might never resolve/reject + first: 60_000, + with: () => + throwError(() => new Error(AccountSwitcherService.incompleteAccountSwitchError)), + }), + ), + ); + + // Initiate the actions required to make account switching happen + await this.accountService.switchAccount(id as UserId); + this.messagingService.send("switchAccount", { userId: id }); // This message should cause switchAccountFinish to be sent + + // Wait until we recieve the switchAccountFinished message + await switchAccountFinishedPromise.catch((err) => { + if ( + err instanceof Error && + err.message === AccountSwitcherService.incompleteAccountSwitchError + ) { + this.logService.warning("message 'switchAccount' never responded."); + return; + } + throw err; + }); + } +} diff --git a/apps/browser/src/auth/popup/account-switching/services/current-account.service.ts b/apps/browser/src/auth/popup/account-switching/services/current-account.service.ts new file mode 100644 index 00000000000..21fc3bdac43 --- /dev/null +++ b/apps/browser/src/auth/popup/account-switching/services/current-account.service.ts @@ -0,0 +1,44 @@ +import { Injectable } from "@angular/core"; +import { Observable, switchMap } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { UserId } from "@bitwarden/common/types/guid"; + +export type CurrentAccount = { + id: UserId; + name: string | undefined; + email: string; + status: AuthenticationStatus; + avatarColor: string; +}; + +@Injectable({ + providedIn: "root", +}) +export class CurrentAccountService { + currentAccount$: Observable; + + constructor( + private accountService: AccountService, + private stateService: StateService, + ) { + this.currentAccount$ = this.accountService.activeAccount$.pipe( + switchMap(async (account) => { + if (account == null) { + return null; + } + const currentAccount: CurrentAccount = { + id: account.id, + name: account.name || account.email, + email: account.email, + status: account.status, + avatarColor: await this.stateService.getAvatarColor({ userId: account.id }), + }; + + return currentAccount; + }), + ); + } +} diff --git a/apps/browser/src/auth/popup/home.component.html b/apps/browser/src/auth/popup/home.component.html index 42fe3106ffb..f70a4c6d030 100644 --- a/apps/browser/src/auth/popup/home.component.html +++ b/apps/browser/src/auth/popup/home.component.html @@ -1,4 +1,4 @@ - +
- +
diff --git a/apps/browser/src/popup/settings/settings.component.html b/apps/browser/src/popup/settings/settings.component.html index 140bf38dee0..f099528918b 100644 --- a/apps/browser/src/popup/settings/settings.component.html +++ b/apps/browser/src/popup/settings/settings.component.html @@ -1,4 +1,4 @@ -
+
@@ -6,7 +6,7 @@ {{ "settings" | i18n }}
-
+

{{ "manage" | i18n }}

diff --git a/apps/browser/src/tools/popup/generator/generator.component.html b/apps/browser/src/tools/popup/generator/generator.component.html index a236c765577..5f722b661ce 100644 --- a/apps/browser/src/tools/popup/generator/generator.component.html +++ b/apps/browser/src/tools/popup/generator/generator.component.html @@ -1,4 +1,4 @@ -
+
-
+
{{ "passwordGeneratorPolicyInEffect" | i18n }} diff --git a/apps/browser/src/tools/popup/send/send-groupings.component.html b/apps/browser/src/tools/popup/send/send-groupings.component.html index e677895b409..edeabd6546a 100644 --- a/apps/browser/src/tools/popup/send/send-groupings.component.html +++ b/apps/browser/src/tools/popup/send/send-groupings.component.html @@ -1,9 +1,9 @@ -
+

{{ "send" | i18n }}

-
+
-
+
{{ "sendDisabledWarning" | i18n }} diff --git a/apps/browser/src/vault/popup/components/vault/current-tab.component.html b/apps/browser/src/vault/popup/components/vault/current-tab.component.html index 4ff7adf4678..c971f6c9371 100644 --- a/apps/browser/src/vault/popup/components/vault/current-tab.component.html +++ b/apps/browser/src/vault/popup/components/vault/current-tab.component.html @@ -1,4 +1,4 @@ -
+

{{ "currentTab" | i18n }}

@@ -11,7 +11,7 @@
-
+
-
+
diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 8ff79c7a151..895ff347f24 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -12,6 +12,7 @@ import { OBSERVABLE_DISK_STORAGE, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { PolicyService as PolicyServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -114,6 +115,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); PlatformUtilsServiceAbstraction, RELOAD_CALLBACK, StateServiceAbstraction, + VaultTimeoutSettingsService, ], }, { diff --git a/libs/angular/src/auth/components/set-pin.component.ts b/libs/angular/src/auth/components/set-pin.component.ts index 2190b111e00..6b5bd40fd35 100644 --- a/libs/angular/src/auth/components/set-pin.component.ts +++ b/libs/angular/src/auth/components/set-pin.component.ts @@ -3,7 +3,6 @@ import { Directive, OnInit } from "@angular/core"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { KeySuffixOptions } from "@bitwarden/common/platform/enums/key-suffix-options.enum"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { ModalRef } from "../../components/modal/modal.ref"; @@ -52,7 +51,6 @@ export class SetPinComponent implements OnInit { } else { await this.stateService.setPinKeyEncryptedUserKey(pinProtectedKey); } - await this.cryptoService.clearDeprecatedKeys(KeySuffixOptions.Pin); this.modalRef.close(true); } diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 542d4ec5214..987e13c2ea5 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -547,4 +547,5 @@ export abstract class StateService { * @param options Defines the storage options for the URL; Defaults to session Storage. */ setDeepLinkRedirectUrl: (url: string, options?: StorageOptions) => Promise; + nextUpActiveUser: () => Promise; } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 65b9ef6ae11..740afe5592b 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -96,7 +96,7 @@ export class StateService< activeAccountUnlocked$ = this.activeAccountUnlockedSubject.asObservable(); private hasBeenInited = false; - private isRecoveredSession = false; + protected isRecoveredSession = false; protected accountDiskCache = new BehaviorSubject>({}); @@ -159,7 +159,7 @@ export class StateService< (await this.storageService.get(keys.authenticatedAccounts)) ?? []; for (const i in state.authenticatedAccounts) { if (i != null) { - await this.syncAccountFromDisk(state.authenticatedAccounts[i]); + state = await this.syncAccountFromDisk(state.authenticatedAccounts[i]); } } const storedActiveUser = await this.storageService.get(keys.activeUserId); @@ -186,25 +186,37 @@ export class StateService< }); } - async syncAccountFromDisk(userId: string) { + async syncAccountFromDisk(userId: string): Promise> { if (userId == null) { return; } - await this.updateState(async (state) => { + const diskAccount = await this.getAccountFromDisk({ userId: userId }); + const state = await this.updateState(async (state) => { if (state.accounts == null) { state.accounts = {}; } state.accounts[userId] = this.createAccount(); - const diskAccount = await this.getAccountFromDisk({ userId: userId }); state.accounts[userId].profile = diskAccount.profile; - // TODO: Temporary update to avoid routing all account status changes through account service for now. - await this.accountService.addAccount(userId as UserId, { - status: AuthenticationStatus.Locked, - name: diskAccount.profile.name, - email: diskAccount.profile.email, - }); return state; }); + + // TODO: Temporary update to avoid routing all account status changes through account service for now. + // The determination of state should be handled by the various services that control those values. + const token = await this.getAccessToken({ userId: userId }); + const autoKey = await this.getUserKeyAutoUnlock({ userId: userId }); + const accountStatus = + token == null + ? AuthenticationStatus.LoggedOut + : autoKey == null + ? AuthenticationStatus.Locked + : AuthenticationStatus.Unlocked; + await this.accountService.addAccount(userId as UserId, { + status: accountStatus, + name: diskAccount.profile.name, + email: diskAccount.profile.email, + }); + + return state; } async addAccount(account: TAccount) { @@ -3033,7 +3045,7 @@ export class StateService< } protected async saveAccountToMemory(account: TAccount): Promise { - if (this.getAccountFromMemory({ userId: account.profile.userId }) !== null) { + if ((await this.getAccountFromMemory({ userId: account.profile.userId })) !== null) { await this.updateState((state) => { return new Promise((resolve) => { state.accounts[account.profile.userId] = account; @@ -3320,10 +3332,9 @@ export class StateService< await this.removeAccountFromSecureStorage(userId); } - protected async dynamicallySetActiveUser() { + async nextUpActiveUser() { const accounts = (await this.state())?.accounts; if (accounts == null || Object.keys(accounts).length < 1) { - await this.setActiveUser(null); return null; } @@ -3338,6 +3349,11 @@ export class StateService< } newActiveUser = null; } + return newActiveUser as UserId; + } + + protected async dynamicallySetActiveUser() { + const newActiveUser = await this.nextUpActiveUser(); await this.setActiveUser(newActiveUser); return newActiveUser; } @@ -3369,20 +3385,23 @@ export class StateService< return state; } - private async setState(state: State): Promise { + private async setState( + state: State, + ): Promise> { await this.memoryStorageService.save(keys.state, state); + return state; } protected async updateState( stateUpdater: (state: State) => Promise>, - ) { - await this.state().then(async (state) => { + ): Promise> { + return await this.state().then(async (state) => { const updatedState = await stateUpdater(state); if (updatedState == null) { throw new Error("Attempted to update state to null value"); } - await this.setState(updatedState); + return await this.setState(updatedState); }); } diff --git a/libs/common/src/platform/services/system.service.ts b/libs/common/src/platform/services/system.service.ts index 5e5d838156d..beb37be73ab 100644 --- a/libs/common/src/platform/services/system.service.ts +++ b/libs/common/src/platform/services/system.service.ts @@ -1,7 +1,9 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, timeout } from "rxjs"; +import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; +import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; import { MessagingService } from "../abstractions/messaging.service"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; import { StateService } from "../abstractions/state.service"; @@ -18,6 +20,7 @@ export class SystemService implements SystemServiceAbstraction { private platformUtilsService: PlatformUtilsService, private reloadCallback: () => Promise = null, private stateService: StateService, + private vaultTimeoutSettingsService: VaultTimeoutSettingsService, ) {} async startProcessReload(authService: AuthService): Promise { @@ -54,6 +57,19 @@ export class SystemService implements SystemServiceAbstraction { if (!biometricLockedFingerprintValidated) { clearInterval(this.reloadInterval); this.reloadInterval = null; + + const currentUser = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); + // Replace current active user if they will be logged out on reload + if (currentUser != null) { + const timeoutAction = await firstValueFrom( + this.vaultTimeoutSettingsService.vaultTimeoutAction$().pipe(timeout(500)), + ); + if (timeoutAction === VaultTimeoutAction.LogOut) { + const nextUser = await this.stateService.nextUpActiveUser(); + await this.stateService.setActiveUser(nextUser); + } + } + this.messagingService.send("reloadProcess"); if (this.reloadCallback != null) { await this.reloadCallback(); diff --git a/libs/components/tailwind.config.js b/libs/components/tailwind.config.js index b9a0cab3c05..987b969e8f0 100644 --- a/libs/components/tailwind.config.js +++ b/libs/components/tailwind.config.js @@ -3,6 +3,7 @@ const config = require("./tailwind.config.base"); config.content = [ "libs/components/src/**/*.{html,ts,mdx}", + "libs/auth/src/**/*.{html,ts,mdx}", "apps/web/src/**/*.{html,ts,mdx}", "bitwarden_license/bit-web/src/**/*.{html,ts,mdx}", ".storybook/preview.tsx",