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 e56a2d5c38..fbb9075156 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,7 +1,7 @@ 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 { Subject, firstValueFrom, map, of, switchMap, takeUntil } from "rxjs"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; @@ -49,7 +49,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { readonly currentAccount$ = this.accountService.activeAccount$.pipe( switchMap((a) => a == null - ? null + ? of(null) : this.authService.activeAccountStatus$.pipe(map((s) => ({ ...a, status: s }))), ), ); @@ -106,12 +106,14 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { }); if (confirmed) { - this.messagingService.send("logout", { userId }); + const result = await this.accountSwitcherService.logoutAccount(userId); + // unlocked logout responses need to be navigated out of the account switcher. + // other responses will be handled by background and app.component + if (result?.status === AuthenticationStatus.Unlocked) { + this.location.back(); + } } - - // 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(["home"]); + this.loading = false; } ngOnDestroy() { diff --git a/apps/browser/src/auth/popup/account-switching/account.component.ts b/apps/browser/src/auth/popup/account-switching/account.component.ts index 3fe83f2414..3f152d61b9 100644 --- a/apps/browser/src/auth/popup/account-switching/account.component.ts +++ b/apps/browser/src/auth/popup/account-switching/account.component.ts @@ -1,10 +1,10 @@ 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 { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AvatarModule } from "@bitwarden/components"; import { AccountSwitcherService, AvailableAccount } from "./services/account-switcher.service"; @@ -21,9 +21,9 @@ export class AccountComponent { constructor( private accountSwitcherService: AccountSwitcherService, - private router: Router, private location: Location, private i18nService: I18nService, + private logService: LogService, ) {} get specialAccountAddId() { @@ -32,15 +32,19 @@ export class AccountComponent { async selectAccount(id: string) { this.loading.emit(true); - await this.accountSwitcherService.selectAccount(id); + let result; + try { + result = await this.accountSwitcherService.selectAccount(id); + } catch (e) { + this.logService.error("Error selecting account", e); + } - if (id === this.specialAccountAddId) { - // 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(["home"]); - } else { + // Navigate out of account switching for unlocked accounts + // locked or logged out account statuses are handled by background and app.component + if (result?.status === AuthenticationStatus.Unlocked) { this.location.back(); } + this.loading.emit(false); } get status() { diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index d27410a5d0..be12d24944 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -186,4 +186,35 @@ describe("AccountSwitcherService", () => { expect(removeListenerSpy).toBeCalledTimes(1); }); }); + + describe("logout", () => { + const userId1 = "1" as UserId; + const userId2 = "2" as UserId; + it("initiates logout", async () => { + let listener: ( + message: { command: string; userId: UserId; status: AuthenticationStatus }, + sender: unknown, + sendResponse: unknown, + ) => void; + jest.spyOn(chrome.runtime.onMessage, "addListener").mockImplementation((addedListener) => { + listener = addedListener; + }); + + const removeListenerSpy = jest.spyOn(chrome.runtime.onMessage, "removeListener"); + + const logoutPromise = accountSwitcherService.logoutAccount(userId1); + + listener( + { command: "switchAccountFinish", userId: userId2, status: AuthenticationStatus.Unlocked }, + undefined, + undefined, + ); + + const result = await logoutPromise; + + expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: userId1 }); + expect(result).toEqual({ newUserId: userId2, status: AuthenticationStatus.Unlocked }); + 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 index e5a3b8f8f5..2650c2db4e 100644 --- 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 @@ -41,7 +41,7 @@ export class AccountSwitcherService { SPECIAL_ADD_ACCOUNT_ID = "addAccount"; availableAccounts$: Observable; - switchAccountFinished$: Observable; + switchAccountFinished$: Observable<{ userId: UserId; status: AuthenticationStatus }>; constructor( private accountService: AccountService, @@ -111,11 +111,11 @@ export class AccountSwitcherService { ); // Create a reusable observable that listens to the switchAccountFinish message and returns the userId from the message - this.switchAccountFinished$ = fromChromeEvent<[message: { command: string; userId: string }]>( - chrome.runtime.onMessage, - ).pipe( + this.switchAccountFinished$ = fromChromeEvent< + [message: { command: string; userId: UserId; status: AuthenticationStatus }] + >(chrome.runtime.onMessage).pipe( filter(([message]) => message.command === "switchAccountFinish"), - map(([message]) => message.userId), + map(([message]) => ({ userId: message.userId, status: message.status })), ); } @@ -127,12 +127,46 @@ export class AccountSwitcherService { if (id === this.SPECIAL_ADD_ACCOUNT_ID) { id = null; } + const userId = id as UserId; // Creates a subscription to the switchAccountFinished observable but further // filters it to only care about the current userId. - const switchAccountFinishedPromise = firstValueFrom( + const switchAccountFinishedPromise = this.listenForSwitchAccountFinish(userId); + + // Initiate the actions required to make account switching happen + await this.accountService.switchAccount(userId); + this.messagingService.send("switchAccount", { userId }); // This message should cause switchAccountFinish to be sent + + // Wait until we receive the switchAccountFinished message + return await switchAccountFinishedPromise; + } + + /** + * + * @param userId the user id to logout + * @returns the userId and status of the that has been switch to due to the logout. null on errors. + */ + async logoutAccount( + userId: UserId, + ): Promise<{ newUserId: UserId; status: AuthenticationStatus } | null> { + // logout creates an account switch to the next up user, which may be null + const switchPromise = this.listenForSwitchAccountFinish(null); + + await this.messagingService.send("logout", { userId }); + + // wait for account switch to happen, the result will be the new user id and status + const result = await switchPromise; + return { newUserId: result.userId, status: result.status }; + } + + // Listens for the switchAccountFinish message and returns the userId from the message + // Optionally filters switchAccountFinish to an expected userId + private listenForSwitchAccountFinish( + expectedUserId: UserId | null, + ): Promise<{ userId: UserId; status: AuthenticationStatus } | null> { + return firstValueFrom( this.switchAccountFinished$.pipe( - filter((userId) => userId === id), + filter(({ userId }) => (expectedUserId ? userId === expectedUserId : true)), 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 @@ -143,20 +177,13 @@ export class AccountSwitcherService { 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) => { + ).catch((err) => { if ( err instanceof Error && err.message === AccountSwitcherService.incompleteAccountSwitchError ) { this.logService.warning("message 'switchAccount' never responded."); - return; + return null; } throw err; }); diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 970b12de50..c3722f2a48 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -100,6 +100,7 @@ import { Message, MessageListener, MessageSender } from "@bitwarden/common/platf // eslint-disable-next-line no-restricted-imports -- Used for dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; import { Lazy } from "@bitwarden/common/platform/misc/lazy"; +import { clearCaches } from "@bitwarden/common/platform/misc/sequentialize"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { AppIdService } from "@bitwarden/common/platform/services/app-id.service"; @@ -815,6 +816,7 @@ export default class MainBackground { logoutCallback, this.billingAccountProfileStateService, this.tokenService, + this.authService, ); this.eventUploadService = new EventUploadService( this.apiService, @@ -1180,6 +1182,7 @@ export default class MainBackground { * Switch accounts to indicated userId -- null is no active user */ async switchAccount(userId: UserId) { + let nextAccountStatus: AuthenticationStatus; try { const currentlyActiveAccount = await firstValueFrom( this.accountService.activeAccount$.pipe(map((account) => account?.id)), @@ -1187,6 +1190,8 @@ export default class MainBackground { // can be removed once password generation history is migrated to state providers await this.stateService.clearDecryptedData(currentlyActiveAccount); await this.accountService.switchAccount(userId); + // Clear sequentialized caches + clearCaches(); if (userId == null) { this.loginEmailService.setRememberEmail(false); @@ -1194,11 +1199,12 @@ export default class MainBackground { await this.refreshBadge(); await this.refreshMenu(); - await this.overlayBackground.updateOverlayCiphers(); + await this.overlayBackground?.updateOverlayCiphers(); // null in popup only contexts + this.messagingService.send("goHome"); return; } - const status = await this.authService.getAuthStatus(userId); + nextAccountStatus = await this.authService.getAuthStatus(userId); const forcePasswordReset = (await firstValueFrom(this.masterPasswordService.forceSetPasswordReason$(userId))) != ForceSetPasswordReason.None; @@ -1206,7 +1212,9 @@ export default class MainBackground { await this.systemService.clearPendingClipboard(); await this.notificationsService.updateConnection(false); - if (status === AuthenticationStatus.Locked) { + if (nextAccountStatus === AuthenticationStatus.LoggedOut) { + this.messagingService.send("goHome"); + } else if (nextAccountStatus === AuthenticationStatus.Locked) { this.messagingService.send("locked", { userId: userId }); } else if (forcePasswordReset) { this.messagingService.send("update-temp-password", { userId: userId }); @@ -1214,11 +1222,14 @@ export default class MainBackground { this.messagingService.send("unlocked", { userId: userId }); await this.refreshBadge(); await this.refreshMenu(); - await this.overlayBackground.updateOverlayCiphers(); + await this.overlayBackground?.updateOverlayCiphers(); // null in popup only contexts await this.syncService.fullSync(false); } } finally { - this.messagingService.send("switchAccountFinish", { userId: userId }); + this.messagingService.send("switchAccountFinish", { + userId: userId, + status: nextAccountStatus, + }); } } @@ -1239,6 +1250,13 @@ export default class MainBackground { await this.eventUploadService.uploadEvents(userBeingLoggedOut); + const newActiveUser = + userBeingLoggedOut === activeUserId + ? await firstValueFrom(this.accountService.nextUpAccount$.pipe(map((a) => a?.id))) + : null; + + await this.switchAccount(newActiveUser); + // HACK: We shouldn't wait for the authentication status to change but instead subscribe to the // authentication status to do various actions. const logoutPromise = firstValueFrom( @@ -1273,11 +1291,6 @@ export default class MainBackground { //Needs to be checked before state is cleaned const needStorageReseed = await this.needsStorageReseed(userId); - const newActiveUser = - userBeingLoggedOut === activeUserId - ? await firstValueFrom(this.accountService.nextUpAccount$.pipe(map((a) => a?.id))) - : null; - await this.stateService.clean({ userId: userBeingLoggedOut }); await this.accountService.clean(userBeingLoggedOut); @@ -1286,16 +1299,10 @@ export default class MainBackground { // HACK: Wait for the user logging outs authentication status to transition to LoggedOut await logoutPromise; - await this.switchAccount(newActiveUser); - if (newActiveUser != null) { - // we have a new active user, do not continue tearing down application - this.messagingService.send("switchAccountFinish"); - } else { - this.messagingService.send("doneLoggingOut", { - expired: expired, - userId: userBeingLoggedOut, - }); - } + this.messagingService.send("doneLoggingOut", { + expired: expired, + userId: userBeingLoggedOut, + }); if (needStorageReseed) { await this.reseedStorage(); @@ -1308,9 +1315,7 @@ export default class MainBackground { } await this.refreshBadge(); await this.mainContextMenuHandler?.noAccess(); - // 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); + await this.notificationsService.updateConnection(false); await this.systemService.clearPendingClipboard(); await this.systemService.startProcessReload(this.authService); } diff --git a/apps/browser/src/platform/services/abstractions/abstract-chrome-storage-api.service.ts b/apps/browser/src/platform/services/abstractions/abstract-chrome-storage-api.service.ts index 259d6f154a..2bdc2fd0d4 100644 --- a/apps/browser/src/platform/services/abstractions/abstract-chrome-storage-api.service.ts +++ b/apps/browser/src/platform/services/abstractions/abstract-chrome-storage-api.service.ts @@ -1,4 +1,4 @@ -import { mergeMap } from "rxjs"; +import { filter, mergeMap } from "rxjs"; import { AbstractStorageService, @@ -34,6 +34,11 @@ export default abstract class AbstractChromeStorageService constructor(protected chromeStorageApi: chrome.storage.StorageArea) { this.updates$ = fromChromeEvent(this.chromeStorageApi.onChanged).pipe( + filter(([changes]) => { + // Our storage services support changing only one key at a time. If more are changed, it's due to + // reseeding storage and we should ignore the changes. + return Object.keys(changes).length === 1; + }), mergeMap(([changes]) => { return Object.entries(changes).map(([key, change]) => { // The `newValue` property isn't on the StorageChange object diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 25fac44450..7e94e84ef5 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -91,13 +91,9 @@ export class AppComponent implements OnInit, OnDestroy { message: this.i18nService.t("loginExpired"), }); } - - // 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(["home"]); }); this.changeDetectorRef.detectChanges(); - } else if (msg.command === "authBlocked") { + } else if (msg.command === "authBlocked" || msg.command === "goHome") { // 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(["home"]); @@ -137,9 +133,6 @@ export class AppComponent implements OnInit, OnDestroy { // 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(["/remove-password"]); - } else if (msg.command === "switchAccountFinish") { - // TODO: unset loading? - // this.loading = false; } else if (msg.command == "update-temp-password") { // 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 diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index cab38965f8..31de198194 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -664,6 +664,7 @@ export class Main { async (expired: boolean) => await this.logout(), this.billingAccountProfileStateService, this.tokenService, + this.authService, ); this.totpService = new TotpService(this.cryptoFunctionService, this.logService); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 806fa6de1b..e77ef8d3f0 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -38,6 +38,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { clearCaches } from "@bitwarden/common/platform/misc/sequentialize"; import { StateEventRunnerService } from "@bitwarden/common/platform/state"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { UserId } from "@bitwarden/common/types/guid"; @@ -396,6 +397,8 @@ export class AppComponent implements OnInit, OnDestroy { this.router.navigate(["/remove-password"]); break; case "switchAccount": { + // Clear sequentialized caches + clearCaches(); if (message.userId != null) { await this.stateService.clearDecryptedData(message.userId); await this.accountService.switchAccount(message.userId); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index ee14d97e68..c812f5fbec 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -631,6 +631,7 @@ const safeProviders: SafeProvider[] = [ LOGOUT_CALLBACK, BillingAccountProfileStateService, TokenServiceAbstraction, + AuthServiceAbstraction, ], }), safeProvider({ diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 6740387ded..8af06b89e7 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -119,6 +119,7 @@ export class AccountServiceImplementation implements InternalAccountService { } async switchAccount(userId: UserId): Promise { + let updateActivity = false; await this.activeAccountIdState.update( (_, accounts) => { if (userId == null) { @@ -129,6 +130,7 @@ export class AccountServiceImplementation implements InternalAccountService { if (accounts?.[userId] == null) { throw new Error("Account does not exist"); } + updateActivity = true; return userId; }, { @@ -139,6 +141,10 @@ export class AccountServiceImplementation implements InternalAccountService { }, }, ); + + if (updateActivity) { + await this.setAccountActivity(userId, new Date()); + } } async setAccountActivity(userId: UserId, lastActivity: Date): Promise { diff --git a/libs/common/src/platform/misc/sequentialize.spec.ts b/libs/common/src/platform/misc/sequentialize.spec.ts index 1dc5f2b2c2..0b79cde387 100644 --- a/libs/common/src/platform/misc/sequentialize.spec.ts +++ b/libs/common/src/platform/misc/sequentialize.spec.ts @@ -1,4 +1,4 @@ -import { sequentialize } from "./sequentialize"; +import { clearCaches, sequentialize } from "./sequentialize"; describe("sequentialize decorator", () => { it("should call the function once", async () => { @@ -100,6 +100,18 @@ describe("sequentialize decorator", () => { allRes.sort(); expect(allRes).toEqual([3, 3, 6, 6, 9, 9]); }); + + describe("clearCaches", () => { + it("should clear all caches", async () => { + const foo = new Foo(); + const promise = Promise.all([foo.bar(1), foo.bar(1)]); + clearCaches(); + await foo.bar(1); + await promise; + // one call for the first two, one for the third after the cache was cleared + expect(foo.calls).toBe(2); + }); + }); }); class Foo { diff --git a/libs/common/src/platform/misc/sequentialize.ts b/libs/common/src/platform/misc/sequentialize.ts index c242114764..852c32db56 100644 --- a/libs/common/src/platform/misc/sequentialize.ts +++ b/libs/common/src/platform/misc/sequentialize.ts @@ -1,3 +1,19 @@ +const caches = new Map>>(); + +const getCache = (obj: any) => { + let cache = caches.get(obj); + if (cache != null) { + return cache; + } + cache = new Map>(); + caches.set(obj, cache); + return cache; +}; + +export function clearCaches() { + caches.clear(); +} + /** * Use as a Decorator on async functions, it will prevent multiple 'active' calls as the same time * @@ -11,17 +27,6 @@ export function sequentialize(cacheKey: (args: any[]) => string) { return (target: any, propertyKey: string | symbol, descriptor: PropertyDescriptor) => { const originalMethod: () => Promise = descriptor.value; - const caches = new Map>>(); - - const getCache = (obj: any) => { - let cache = caches.get(obj); - if (cache != null) { - return cache; - } - cache = new Map>(); - caches.set(obj, cache); - return cache; - }; return { value: function (...args: any[]) { diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 174da701bd..537245459e 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -28,7 +28,7 @@ import { DerivedState, StateProvider, } from "../../platform/state"; -import { CipherId, CollectionId, OrganizationId } from "../../types/guid"; +import { CipherId, CollectionId, OrganizationId, UserId } from "../../types/guid"; import { UserKey, OrgKey } from "../../types/key"; import { CipherService as CipherServiceAbstraction } from "../abstractions/cipher.service"; import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service"; @@ -136,11 +136,12 @@ export class CipherService implements CipherServiceAbstraction { } async setDecryptedCipherCache(value: CipherView[]) { + const userId = await firstValueFrom(this.stateProvider.activeUserId$); // Sometimes we might prematurely decrypt the vault and that will result in no ciphers - // if we cache it then we may accidentially return it when it's not right, we'd rather try decryption again. + // if we cache it then we may accidentally return it when it's not right, we'd rather try decryption again. // We still want to set null though, that is the indicator that the cache isn't valid and we should do decryption. if (value == null || value.length !== 0) { - await this.setDecryptedCiphers(value); + await this.setDecryptedCiphers(value, userId); } if (this.searchService != null) { if (value == null) { @@ -151,15 +152,16 @@ export class CipherService implements CipherServiceAbstraction { } } - private async setDecryptedCiphers(value: CipherView[]) { + private async setDecryptedCiphers(value: CipherView[], userId: UserId) { const cipherViews: { [id: string]: CipherView } = {}; value?.forEach((c) => { cipherViews[c.id] = c; }); - await this.decryptedCiphersState.update(() => cipherViews); + await this.stateProvider.setUserState(DECRYPTED_CIPHERS, cipherViews, userId); } - async clearCache(userId?: string): Promise { + async clearCache(userId?: UserId): Promise { + userId ??= await firstValueFrom(this.stateProvider.activeUserId$); await this.clearDecryptedCiphersState(userId); } @@ -524,6 +526,7 @@ export class CipherService implements CipherServiceAbstraction { } async updateLastUsedDate(id: string): Promise { + const userId = await firstValueFrom(this.stateProvider.activeUserId$); let ciphersLocalData = await firstValueFrom(this.localData$); if (!ciphersLocalData) { @@ -553,10 +556,11 @@ export class CipherService implements CipherServiceAbstraction { break; } } - await this.setDecryptedCiphers(decryptedCipherCache); + await this.setDecryptedCiphers(decryptedCipherCache, userId); } async updateLastLaunchedDate(id: string): Promise { + const userId = await firstValueFrom(this.stateProvider.activeUserId$); let ciphersLocalData = await firstValueFrom(this.localData$); if (!ciphersLocalData) { @@ -586,7 +590,7 @@ export class CipherService implements CipherServiceAbstraction { break; } } - await this.setDecryptedCiphers(decryptedCipherCache); + await this.setDecryptedCiphers(decryptedCipherCache, userId); } async saveNeverDomain(domain: string): Promise { @@ -833,12 +837,18 @@ export class CipherService implements CipherServiceAbstraction { await this.updateEncryptedCipherState(() => ciphers); } + /** + * Updates ciphers for the currently active user. Inactive users can only clear all ciphers, for now. + * @param update update callback for encrypted cipher data + * @returns + */ private async updateEncryptedCipherState( update: (current: Record) => Record, ): Promise> { + const userId = await firstValueFrom(this.stateProvider.activeUserId$); // Store that we should wait for an update to return any ciphers await this.ciphersExpectingUpdate.forceValue(true); - await this.clearDecryptedCiphersState(); + await this.clearDecryptedCiphersState(userId); const [, updatedCiphers] = await this.encryptedCiphersState.update((current) => { const result = update(current ?? {}); return result; @@ -846,7 +856,8 @@ export class CipherService implements CipherServiceAbstraction { return updatedCiphers; } - async clear(userId?: string): Promise { + async clear(userId?: UserId): Promise { + userId ??= await firstValueFrom(this.stateProvider.activeUserId$); await this.clearEncryptedCiphersState(userId); await this.clearCache(userId); } @@ -1464,12 +1475,12 @@ export class CipherService implements CipherServiceAbstraction { } } - private async clearEncryptedCiphersState(userId?: string) { - await this.encryptedCiphersState.update(() => ({})); + private async clearEncryptedCiphersState(userId: UserId) { + await this.stateProvider.setUserState(ENCRYPTED_CIPHERS, {}, userId); } - private async clearDecryptedCiphersState(userId?: string) { - await this.setDecryptedCiphers(null); + private async clearDecryptedCiphersState(userId: UserId) { + await this.setDecryptedCiphers(null, userId); this.clearSortedCiphers(); } diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index 793bcf2437..b591a61e86 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, map, of, switchMap } from "rxjs"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; @@ -12,10 +12,12 @@ import { PolicyData } from "../../../admin-console/models/data/policy.data"; import { ProviderData } from "../../../admin-console/models/data/provider.data"; import { PolicyResponse } from "../../../admin-console/models/response/policy.response"; import { AccountService } from "../../../auth/abstractions/account.service"; +import { AuthService } from "../../../auth/abstractions/auth.service"; import { AvatarService } from "../../../auth/abstractions/avatar.service"; import { KeyConnectorService } from "../../../auth/abstractions/key-connector.service"; import { InternalMasterPasswordServiceAbstraction } from "../../../auth/abstractions/master-password.service.abstraction"; import { TokenService } from "../../../auth/abstractions/token.service"; +import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { DomainSettingsService } from "../../../autofill/services/domain-settings.service"; import { BillingAccountProfileStateService } from "../../../billing/abstractions/account/billing-account-profile-state.service"; @@ -74,6 +76,7 @@ export class SyncService implements SyncServiceAbstraction { private logoutCallback: (expired: boolean) => Promise, private billingAccountProfileStateService: BillingAccountProfileStateService, private tokenService: TokenService, + private authService: AuthService, ) {} async getLastSync(): Promise { @@ -247,7 +250,19 @@ export class SyncService implements SyncServiceAbstraction { async syncUpsertSend(notification: SyncSendNotification, isEdit: boolean): Promise { this.syncStarted(); - if (await this.stateService.getIsAuthenticated()) { + const [activeUserId, status] = await firstValueFrom( + this.accountService.activeAccount$.pipe( + switchMap((a) => { + if (a == null) { + of([null, AuthenticationStatus.LoggedOut]); + } + return this.authService.authStatusFor$(a.id).pipe(map((s) => [a.id, s])); + }), + ), + ); + // Process only notifications for currently active user when user is not logged out + // TODO: once send service allows data manipulation of non-active users, this should process any received notification + if (activeUserId === notification.userId && status !== AuthenticationStatus.LoggedOut) { try { const localSend = await firstValueFrom(this.sendService.get$(notification.id)); if ( @@ -361,15 +376,14 @@ export class SyncService implements SyncServiceAbstraction { private async setForceSetPasswordReasonIfNeeded(profileResponse: ProfileResponse) { // The `forcePasswordReset` flag indicates an admin has reset the user's password and must be updated if (profileResponse.forcePasswordReset) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setForceSetPasswordReason( ForceSetPasswordReason.AdminForcePasswordReset, - userId, + profileResponse.id, ); } const userDecryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(profileResponse.id), ); if (userDecryptionOptions === null || userDecryptionOptions === undefined) {