1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-21 16:18:28 +01:00

Auth/PM-3859 - Desktop - Create lock mechanism to prevent app menu redraw on sync complete when user logging out (#6920)

* PM-3859 - Desktop App Comp - Build lock mechanism for update app menu which doesn't try to update the menu for users who are being logged out which was causing errors (primary scenario was triggered by logging in on desktop with a TDE user w/out a MP, triggering a sync, then hitting the command to lock the vault right after which would trigger a log out while the sync was still in process.  Then, while the log out was in process, the sync would try and trigger an update to the app menu but it would error as some of the user's state had already been cleaned up)

* PM-3859 - App comp - remove use of promise.all to prevent any race conditions from causing intermittent logout errors with state being cleared and then values trying to be set on the cleared state (I observed setMasterKey get called after state account.keys was cleared - received error when attempting to set value on undefined).

* PM-3859 - Desktop Vault Items Component - on log out, if you were on the vault screen, the loss of focus on the vault search text box would trigger a search 200 ms after log out had been triggered.  This would eventually attempt to set an undefined master key (VaultItemsComponent.doSearch() --> cipherService.getAllDecrypted() --> cryptoService.getUserKeyWithLegacySupport() --> cryptoService.getMasterKey() --> cryptoService.setMasterKey()).  However, at this point, the account had been cleared as part of the log out process and an error would be thrown in the state service for trying to set account.keys.masterKey to undefined when the account and account.keys were undefined. These changes prevent the search from firing until the value changes and also prevents setMasterKey from being called if it is undefined.
This commit is contained in:
Jared Snider 2024-01-02 17:22:17 -05:00 committed by GitHub
parent 4784ab12e5
commit 1fdc6629e3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 18 deletions

View File

@ -113,6 +113,8 @@ export class AppComponent implements OnInit, OnDestroy {
private destroy$ = new Subject<void>();
private accountCleanUpInProgress: { [userId: string]: boolean } = {};
constructor(
private broadcasterService: BroadcasterService,
private folderService: InternalFolderService,
@ -495,7 +497,11 @@ export class AppComponent implements OnInit, OnDestroy {
} else {
const accounts: { [userId: string]: MenuAccount } = {};
for (const i in stateAccounts) {
if (i != null && stateAccounts[i]?.profile?.userId != null) {
if (
i != null &&
stateAccounts[i]?.profile?.userId != null &&
!this.isAccountCleanUpInProgress(stateAccounts[i].profile.userId) // skip accounts that are being cleaned up
) {
const userId = stateAccounts[i].profile.userId;
const availableTimeoutActions = await firstValueFrom(
this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(userId),
@ -525,22 +531,31 @@ export class AppComponent implements OnInit, OnDestroy {
private async logOut(expired: boolean, userId?: string) {
const userBeingLoggedOut = await this.stateService.getUserId({ userId: userId });
await Promise.all([
this.eventUploadService.uploadEvents(userBeingLoggedOut),
this.syncService.setLastSync(new Date(0), userBeingLoggedOut),
this.cryptoService.clearKeys(userBeingLoggedOut),
this.settingsService.clear(userBeingLoggedOut),
this.cipherService.clear(userBeingLoggedOut),
this.folderService.clear(userBeingLoggedOut),
this.collectionService.clear(userBeingLoggedOut),
this.passwordGenerationService.clear(userBeingLoggedOut),
this.vaultTimeoutSettingsService.clear(userBeingLoggedOut),
this.policyService.clear(userBeingLoggedOut),
this.keyConnectorService.clear(),
]);
const preLogoutActiveUserId = this.activeUserId;
await this.stateService.clean({ userId: userBeingLoggedOut });
// Mark account as being cleaned up so that the updateAppMenu logic (executed on syncCompleted)
// doesn't attempt to update a user that is being logged out as we will manually
// call updateAppMenu when the logout is complete.
this.startAccountCleanUp(userBeingLoggedOut);
let preLogoutActiveUserId;
try {
await this.eventUploadService.uploadEvents(userBeingLoggedOut);
await this.syncService.setLastSync(new Date(0), userBeingLoggedOut);
await this.cryptoService.clearKeys(userBeingLoggedOut);
await this.settingsService.clear(userBeingLoggedOut);
await this.cipherService.clear(userBeingLoggedOut);
await this.folderService.clear(userBeingLoggedOut);
await this.collectionService.clear(userBeingLoggedOut);
await this.passwordGenerationService.clear(userBeingLoggedOut);
await this.vaultTimeoutSettingsService.clear(userBeingLoggedOut);
await this.policyService.clear(userBeingLoggedOut);
await this.keyConnectorService.clear();
preLogoutActiveUserId = this.activeUserId;
await this.stateService.clean({ userId: userBeingLoggedOut });
} finally {
this.finishAccountCleanUp(userBeingLoggedOut);
}
if (this.activeUserId == null) {
this.router.navigate(["login"]);
@ -675,4 +690,19 @@ export class AppComponent implements OnInit, OnDestroy {
const action = await this.stateService.getVaultTimeoutAction({ userId: userId });
return [timeout, action];
}
// Mark an account's clean up as started
private startAccountCleanUp(userId: string): void {
this.accountCleanUpInProgress[userId] = true;
}
// Mark an account's clean up as finished
private finishAccountCleanUp(userId: string): void {
this.accountCleanUpInProgress[userId] = false;
}
// Check if an account's clean up is in progress
private isAccountCleanUpInProgress(userId: string): boolean {
return this.accountCleanUpInProgress[userId] === true;
}
}

View File

@ -1,4 +1,5 @@
import { Component } from "@angular/core";
import { distinctUntilChanged } from "rxjs";
import { VaultItemsComponent as BaseVaultItemsComponent } from "@bitwarden/angular/vault/components/vault-items.component";
import { SearchService } from "@bitwarden/common/abstractions/search.service";
@ -21,7 +22,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent {
super(searchService, cipherService);
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
searchBarService.searchText$.subscribe((searchText) => {
searchBarService.searchText$.pipe(distinctUntilChanged()).subscribe((searchText) => {
this.searchText = searchText;
this.search(200);
});

View File

@ -161,7 +161,11 @@ export class CryptoService implements CryptoServiceAbstraction {
let masterKey = await this.stateService.getMasterKey({ userId: userId });
if (!masterKey) {
masterKey = (await this.stateService.getCryptoMasterKey({ userId: userId })) as MasterKey;
await this.setMasterKey(masterKey, userId);
// if master key was null/undefined and getCryptoMasterKey also returned null/undefined,
// don't set master key as it is unnecessary
if (masterKey) {
await this.setMasterKey(masterKey, userId);
}
}
return masterKey;
}