vaultwarden
/
bitwarden-estensione-browser
mirror of https://github.com/bitwarden/browser
1
0
Fork 0
Code Issues 1 Releases Wiki Activity

[PM-6991] Improve reactivity of cipherViews$ observable in cipher service (#11141) 

* [PM-6991] Remove self reference to cipherViews$

* [PM-6991] Update cipherViews$ observable to be reactive to encrypted ciphers and localData

* [PM-6991] Use the cipherViews$ observable in the Web vault

* [PM-6991] Update the vault filter service to use cipherViews$ observable

* [PM-6991] Add deprecation notice to getAllDecrypted

* [PM-6991] Ensure cipherViews$ emits an empty array whenever the decrypted cipher cache is cleared

* [PM-6991] Fix cipher service test

* [PM-6991] Use shareReplay instead of share

* [PM-6991] Remove ciphersExpectingUpdate and replace with a null filter instead for cipherViews$

* [PM-6991] Skip refreshing on null cipherViews$ to avoid flashing an empty vault after modifying ciphers
This commit is contained in:
Shane Melton 2024-10-10 14:54:23 -07:00 committed by GitHub
parent c221efd09a
commit 9d163550fd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 65 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts
View File

@ -35,6 +35,7 @@ describe("vault filter service", () => {
let organizations: ReplaySubject<Organization[]>;
let folderViews: ReplaySubject<FolderView[]>;
let collectionViews: ReplaySubject<CollectionView[]>;
let cipherViews: ReplaySubject<CipherView[]>;
let personalOwnershipPolicy: ReplaySubject<boolean>;
let singleOrgPolicy: ReplaySubject<boolean>;
let stateProvider: FakeStateProvider;
@ -57,6 +58,7 @@ describe("vault filter service", () => {
organizations = new ReplaySubject<Organization[]>(1);
folderViews = new ReplaySubject<FolderView[]>(1);
collectionViews = new ReplaySubject<CollectionView[]>(1);
cipherViews = new ReplaySubject<CipherView[]>(1);
personalOwnershipPolicy = new ReplaySubject<boolean>(1);
singleOrgPolicy = new ReplaySubject<boolean>(1);
@ -69,6 +71,7 @@ describe("vault filter service", () => {
policyService.policyAppliesToActiveUser$
.calledWith(PolicyType.SingleOrg)
.mockReturnValue(singleOrgPolicy);
cipherService.cipherViews$ = cipherViews;
vaultFilterService = new VaultFilterService(
organizationService,
@ -162,7 +165,7 @@ describe("vault filter service", () => {
createCipherView("1", "org test id", "folder test id"),
createCipherView("2", "non matching org id", "non matching folder id"),
];
cipherService.getAllDecrypted.mockResolvedValue(storedCiphers);
cipherViews.next(storedCiphers);
const storedFolders = [
createFolderView("folder test id", "test"),
@ -191,6 +194,7 @@ describe("vault filter service", () => {
createFolderView("Folder 3 Id", "Folder 1/Folder 3"),
];
folderViews.next(storedFolders);
cipherViews.next([]);
const result = await firstValueFrom(vaultFilterService.folderTree$);

9
apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts
View File

@ -25,6 +25,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
import { CipherType } from "@bitwarden/common/vault/enums";
import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
import { ServiceUtils } from "@bitwarden/common/vault/service-utils";
import { COLLAPSED_GROUPINGS } from "@bitwarden/common/vault/services/key-state/collapsed-groupings.state";
@ -55,9 +56,9 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
protected _organizationFilter = new BehaviorSubject<Organization>(null);
filteredFolders$: Observable<FolderView[]> = this.folderService.folderViews$.pipe(
combineLatestWith(this._organizationFilter),
switchMap(([folders, org]) => {
return this.filterFolders(folders, org);
combineLatestWith(this.cipherService.cipherViews$, this._organizationFilter),
switchMap(([folders, ciphers, org]) => {
return this.filterFolders(folders, ciphers, org);
}),
);
folderTree$: Observable<TreeNode<FolderFilter>> = this.filteredFolders$.pipe(
@ -231,6 +232,7 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
protected async filterFolders(
storedFolders: FolderView[],
ciphers: CipherView[],
org?: Organization,
): Promise<FolderView[]> {
// If no org or "My Vault" is selected, show all folders
@ -239,7 +241,6 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
}
// Otherwise, show only folders that have ciphers from the selected org and the "no folder" folder
const ciphers = await this.cipherService.getAllDecrypted();
const orgCiphers = ciphers.filter((c) => c.organizationId == org?.id);
return storedFolders.filter(
(f) => orgCiphers.some((oc) => oc.folderId == f.id) || f.id == null,

2
apps/web/src/app/vault/individual-vault/vault.component.ts
View File

@ -281,7 +281,7 @@ export class VaultComponent implements OnInit, OnDestroy {
this.currentSearchText$ = this.route.queryParams.pipe(map((queryParams) => queryParams.search));
const ciphers$ = combineLatest([
Utils.asyncToObservable(() => this.cipherService.getAllDecrypted()),
this.cipherService.cipherViews$.pipe(filter((c) => c !== null)),
filter$,
this.currentSearchText$,
]).pipe(

2
libs/common/src/vault/abstractions/cipher.service.ts
View File

@ -17,7 +17,7 @@ import { FieldView } from "../models/view/field.view";
import { AddEditCipherInfo } from "../types/add-edit-cipher-info";
export abstract class CipherService implements UserKeyRotationDataProvider<CipherWithIdRequest> {
cipherViews$: Observable<Record<CipherId, CipherView>>;
cipherViews$: Observable<CipherView[]>;
ciphers$: Observable<Record<CipherId, CipherData>>;
localData$: Observable<Record<CipherId, LocalData>>;
/**

4
libs/common/src/vault/services/cipher.service.spec.ts
View File

@ -1,5 +1,5 @@
import { mock } from "jest-mock-extended";
import { BehaviorSubject, of } from "rxjs";
import { BehaviorSubject, map, of } from "rxjs";
import { BulkEncryptService } from "@bitwarden/common/platform/abstractions/bulk-encrypt.service";
@ -381,7 +381,7 @@ describe("Cipher Service", () => {
Cipher1: cipher1,
Cipher2: cipher2,
});
cipherService.cipherViews$ = decryptedCiphers;
cipherService.cipherViews$ = decryptedCiphers.pipe(map((ciphers) => Object.values(ciphers)));
encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(32));
encryptedKey = new EncString("Re-encrypted Cipher Key");

83
libs/common/src/vault/services/cipher.service.ts
View File

@ -1,4 +1,14 @@
import { firstValueFrom, map, Observable, skipWhile, switchMap } from "rxjs";
import {
combineLatest,
filter,
firstValueFrom,
map,
merge,
Observable,
shareReplay,
Subject,
switchMap,
} from "rxjs";
import { SemVer } from "semver";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@ -24,13 +34,7 @@ import Domain from "../../platform/models/domain/domain-base";
import { EncArrayBuffer } from "../../platform/models/domain/enc-array-buffer";
import { EncString } from "../../platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key";
import {
ActiveUserState,
CIPHERS_MEMORY,
DeriveDefinition,
DerivedState,
StateProvider,
} from "../../platform/state";
import { ActiveUserState, StateProvider } from "../../platform/state";
import { CipherId, CollectionId, OrganizationId, UserId } from "../../types/guid";
import { OrgKey, UserKey } from "../../types/key";
import { CipherService as CipherServiceAbstraction } from "../abstractions/cipher.service";
@ -81,14 +85,25 @@ export class CipherService implements CipherServiceAbstraction {
private sortedCiphersCache: SortedCiphersCache = new SortedCiphersCache(
this.sortCiphersByLastUsed,
);
private ciphersExpectingUpdate: DerivedState<boolean>;
/**
* Observable that forces the `cipherViews$` observable to re-emit with the provided value.
* Used to let subscribers of `cipherViews$` know that the decrypted ciphers have been cleared for the active user.
* @private
*/
private forceCipherViews$: Subject<CipherView[]> = new Subject<CipherView[]>();
localData$: Observable<Record<CipherId, LocalData>>;
ciphers$: Observable<Record<CipherId, CipherData>>;
cipherViews$: Observable<Record<CipherId, CipherView>>;
viewFor$(id: CipherId) {
return this.cipherViews$.pipe(map((views) => views[id]));
}
/**
* Observable that emits an array of decrypted ciphers for the active user.
* This observable will not emit until the encrypted ciphers have either been loaded from state or after sync.
*
* A `null` value indicates that the latest encrypted ciphers have not been decrypted yet and that
* decryption is in progress. The latest decrypted ciphers will be emitted once decryption is complete.
*
*/
cipherViews$: Observable<CipherView[] | null>;
addEditCipherInfo$: Observable<AddEditCipherInfo>;
private localDataState: ActiveUserState<Record<CipherId, LocalData>>;
@ -115,23 +130,16 @@ export class CipherService implements CipherServiceAbstraction {
this.encryptedCiphersState = this.stateProvider.getActive(ENCRYPTED_CIPHERS);
this.decryptedCiphersState = this.stateProvider.getActive(DECRYPTED_CIPHERS);
this.addEditCipherInfoState = this.stateProvider.getActive(ADD_EDIT_CIPHER_INFO_KEY);
this.ciphersExpectingUpdate = this.stateProvider.getDerived(
this.encryptedCiphersState.state$,
new DeriveDefinition(CIPHERS_MEMORY, "ciphersExpectingUpdate", {
derive: (_: Record<CipherId, CipherData>) => false,
deserializer: (value) => value,
}),
{},
);
this.localData$ = this.localDataState.state$.pipe(map((data) => data ?? {}));
// First wait for ciphersExpectingUpdate to be false before emitting ciphers
this.ciphers$ = this.ciphersExpectingUpdate.state$.pipe(
skipWhile((expectingUpdate) => expectingUpdate),
switchMap(() => this.encryptedCiphersState.state$),
map((ciphers) => ciphers ?? {}),
this.ciphers$ = this.encryptedCiphersState.state$.pipe(map((ciphers) => ciphers ?? {}));
// Decrypted ciphers depend on both ciphers and local data and need to be updated when either changes
this.cipherViews$ = combineLatest([this.encryptedCiphersState.state$, this.localData$]).pipe(
filter(([ciphers]) => ciphers != null), // Skip if ciphers haven't been loaded yor synced yet
switchMap(() => merge(this.forceCipherViews$, this.getAllDecrypted())),
shareReplay({ bufferSize: 1, refCount: true }),
);
this.cipherViews$ = this.decryptedCiphersState.state$.pipe(map((views) => views ?? {}));
this.addEditCipherInfo$ = this.addEditCipherInfoState.state$;
}
@ -160,8 +168,14 @@ export class CipherService implements CipherServiceAbstraction {
}
async clearCache(userId?: UserId): Promise<void> {
userId ??= await firstValueFrom(this.stateProvider.activeUserId$);
const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$);
userId ??= activeUserId;
await this.clearDecryptedCiphersState(userId);
// Force the cipherView$ observable (which always tracks the active user) to re-emit
if (userId == activeUserId) {
this.forceCipherViews$.next(null);
}
}
async encrypt(
@ -354,6 +368,11 @@ export class CipherService implements CipherServiceAbstraction {
return response;
}
/**
* Decrypts all ciphers for the active user and caches them in memory. If the ciphers have already been decrypted and
* cached, the cached ciphers are returned.
* @deprecated Use `cipherViews$` observable instead
*/
@sequentialize(() => "getAllDecrypted")
async getAllDecrypted(): Promise<CipherView[]> {
let decCiphers = await this.getDecryptedCiphers();
@ -375,7 +394,9 @@ export class CipherService implements CipherServiceAbstraction {
}
private async getDecryptedCiphers() {
return Object.values(await firstValueFrom(this.cipherViews$));
return Object.values(
await firstValueFrom(this.decryptedCiphersState.state$.pipe(map((c) => c ?? {}))),
);
}
private async decryptCiphers(ciphers: Cipher[], userId: UserId) {
@ -932,8 +953,6 @@ export class CipherService implements CipherServiceAbstraction {
userId: UserId = null,
): Promise<Record<CipherId, CipherData>> {
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(userId);
const updatedCiphers = await this.stateProvider
.getUser(userId, ENCRYPTED_CIPHERS)
@ -1254,7 +1273,7 @@ export class CipherService implements CipherServiceAbstraction {
let encryptedCiphers: CipherWithIdRequest[] = [];
const ciphers = await this.getAllDecrypted();
const ciphers = await firstValueFrom(this.cipherViews$);
if (!ciphers) {
return encryptedCiphers;
}