Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
} from "rxjs";
import {
catchError,
concatMap,
debounceTime,
distinctUntilChanged,
filter,
Expand All @@ -25,6 +24,7 @@ import {
switchMap,
take,
takeUntil,
tap,
} from "rxjs/operators";

import { CollectionAdminService, CollectionService } from "@bitwarden/admin-console/common";
Expand Down Expand Up @@ -280,8 +280,25 @@ export class VaultComponent implements OnInit, OnDestroy {
this.allCollectionsWithoutUnassigned$ = this.refreshingSubject$.pipe(
filter((refreshing) => refreshing),
switchMap(() => combineLatest([this.organizationId$, this.userId$])),
switchMap(([orgId, userId]) =>
this.collectionAdminService.collectionAdminViews$(orgId, userId),
switchMap(
([orgId, userId]) =>
// Run collection decryption outside Angular's zone so the setTimeout yields
// in our chunked decryptMany loop don't each trigger a full CD pass.
new Observable<CollectionAdminView[]>((observer) => {
void this.ngZone.runOutsideAngular(async () => {
try {
const result = await firstValueFrom(
this.collectionAdminService.collectionAdminViews$(orgId, userId),
);
this.ngZone.run(() => {
observer.next(result);
observer.complete();
});
} catch (e) {
this.ngZone.run(() => observer.error(e));
}
});
}),
),
Comment thread
JaredScar marked this conversation as resolved.
shareReplay({ refCount: true, bufferSize: 1 }),
);
Expand All @@ -304,7 +321,27 @@ export class VaultComponent implements OnInit, OnDestroy {
);

this.nestedCollections$ = this.allCollections$.pipe(
map((collections) => getNestedCollectionTree(collections)),
// Run the O(NΒ²) nestedTraverse outside Angular's zone so:
// 1. The deferred setTimeout does not trigger a spurious CD run when it fires.
// 2. The tree-build itself (potentially hundreds of ms for 8K collections) does
// not block Angular's CD scheduler.
// ngZone.run() re-enters the zone exactly once when the result is ready.
switchMap(
(collections) =>
new Observable<TreeNode<CollectionAdminView>[]>((observer) => {
const cancel = this.ngZone.runOutsideAngular(() => {
const id = setTimeout(() => {
const result = getNestedCollectionTree(collections);
this.ngZone.run(() => {
observer.next(result);
observer.complete();
});
}, 0);
return () => clearTimeout(id);
});
return cancel;
}),
),
shareReplay({ refCount: true, bufferSize: 1 }),
);

Expand All @@ -325,29 +362,50 @@ export class VaultComponent implements OnInit, OnDestroy {
if (!this.showAddAccessToggle || organization) {
this.addAccessToggle(0);
}
let ciphers;

// Restricted providers (who are not members) do not have access org cipher endpoint below
// Return early to avoid 404 response
if (!organization.isMember && organization.isProviderUser) {
return [];
}

// If the user can edit all ciphers for the organization then fetch them ALL.
if (organization.canEditAllCiphers) {
ciphers = await this.cipherService.getAllFromApiForOrganization(organization.id);
ciphers.forEach((c) => (c.edit = true));
} else {
// Otherwise, only fetch ciphers they have access to (includes unassigned for admins).
ciphers = await this.cipherService.getManyFromApiForOrganization(organization.id);
}

// Filter out restricted ciphers before indexing
ciphers = ciphers.filter(
(cipher) => !this.restrictedItemTypesService.isCipherRestricted(cipher, restricted),
);
// Run the heavy fetch β†’ decrypt β†’ index pipeline outside Angular's zone.
//
// Our chunked decryption and indexing loops yield via setTimeout(0) between
// batches to keep the browser responsive. However, zone.js intercepts every
// setTimeout callback and triggers a full Angular change-detection pass. With
// Default CD strategy that can mean 40+ full-tree scans during loading β€” each
// one blocking the main thread for tens of milliseconds. Running outside the
// zone means the yields genuinely free the browser without triggering CD.
// Angular gets a single CD run when this promise resolves and the observable
// emits the completed cipher array.
const ciphers = await new Promise<CipherView[]>((resolve, reject) => {
void this.ngZone.runOutsideAngular(async () => {
try {
let result: CipherView[];

// If the user can edit all ciphers for the organization then fetch them ALL.
if (organization.canEditAllCiphers) {
result = await this.cipherService.getAllFromApiForOrganization(organization.id);
result.forEach((c) => (c.edit = true));
} else {
// Otherwise, only fetch ciphers they have access to (includes unassigned for admins).
result = await this.cipherService.getManyFromApiForOrganization(organization.id);
}

// Filter out restricted ciphers before indexing
result = result.filter(
(cipher) => !this.restrictedItemTypesService.isCipherRestricted(cipher, restricted),
);

await this.searchService.indexCiphers(userId, result, organization.id);
resolve(result);
} catch (e) {
reject(e);
}
});
});

await this.searchService.indexCiphers(userId, ciphers, organization.id);
return ciphers;
}),
shareReplay({ refCount: true, bufferSize: 1 }),
Expand Down Expand Up @@ -391,7 +449,7 @@ export class VaultComponent implements OnInit, OnDestroy {
this.userId$,
]).pipe(
filter(([ciphers, filter]) => ciphers != undefined && filter != undefined),
concatMap(async ([ciphers, filter, searchText, showCollectionAccessRestricted, userId]) => {
switchMap(async ([ciphers, filter, searchText, showCollectionAccessRestricted, userId]) => {
if (filter.collectionId === undefined && filter.type === undefined) {
return [];
}
Expand All @@ -405,6 +463,7 @@ export class VaultComponent implements OnInit, OnDestroy {
const filterFunction = createFilterFunction(filter);

if (await this.searchService.isSearchable(userId, searchText)) {
// Search results are already ranked by Lunr relevance β€” preserve that order.
return await this.searchService.searchCiphers<CipherView>(
userId,
searchText,
Expand All @@ -413,7 +472,11 @@ export class VaultComponent implements OnInit, OnDestroy {
);
}

return ciphers.filter(filterFunction);
// Sort the filtered subset here rather than relying solely on the pre-sorted
// allCiphers$ array. The filtered set is a small fraction of allCiphers$ (one
// collection's worth, not all 80K), so this sort costs <1ms and ensures correct
// display order even during the allCiphers$ sort phase.
return ciphers.filter(filterFunction).sort(this.cipherService.getLocaleSortingFunction());
}),
shareReplay({ refCount: true, bufferSize: 1 }),
);
Expand Down Expand Up @@ -455,16 +518,15 @@ export class VaultComponent implements OnInit, OnDestroy {
this.organization$,
]).pipe(
filter(([collections, filter]) => collections != undefined && filter != undefined),
concatMap(
switchMap(
async ([collections, filter, searchText, addAccessStatus, userId, organization]) => {
if (
filter.collectionId === Unassigned ||
(filter.collectionId === undefined && filter.type !== undefined)
) {
return [];
return { collections: [] as CollectionAdminView[], showToggle: false };
}

this.showAddAccessToggle = false;
let searchableCollectionNodes: TreeNode<CollectionAdminView>[] = [];
if (filter.collectionId === undefined || filter.collectionId === All) {
searchableCollectionNodes = collections;
Expand Down Expand Up @@ -496,17 +558,24 @@ export class VaultComponent implements OnInit, OnDestroy {
}

// Add access toggle is only shown if allowAdminAccessToAllCollectionItems is false and there are unmanaged collections the user can edit
this.showAddAccessToggle =
const showToggle =
!organization.allowAdminAccessToAllCollectionItems &&
organization.canEditUnmanagedCollections &&
collectionsToReturn.some((c) => c.unmanaged);

if (addAccessStatus === 1 && this.showAddAccessToggle) {
if (addAccessStatus === 1 && showToggle) {
collectionsToReturn = collectionsToReturn.filter((c) => c.unmanaged);
}
return collectionsToReturn;
return { collections: collectionsToReturn, showToggle };
},
),
// Separate the side effect (updating component state) from the data computation above.
// tap is the designated operator for side effects in RxJS pipelines, making the data
// flow explicit and preventing accidental change-detection triggers mid-pipeline.
tap(({ showToggle }) => {
this.showAddAccessToggle = showToggle;
}),
map(({ collections }) => collections),
takeUntil(this.destroy$),
shareReplay({ refCount: true, bufferSize: 1 }),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
</tr>
</ng-container>
<ng-template body let-rows$>
<ng-container *cdkVirtualFor="let item of rows$; templateCacheSize: 0">
<ng-container *cdkVirtualFor="let item of rows$; templateCacheSize: 20">
@if (item.collection) {
<tr
bitRow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ export class VaultItemsComponent<C extends CipherViewLike> {
protected canRestoreSelected$: Observable<boolean>;
protected disableMenu$: Observable<boolean>;
private restrictedTypes: RestrictedCipherType[] = [];
/**
* A Set of collection IDs where the current user has the "manage" permission. Built once per
* refreshItems() call (O(K)) so that canManageCollection() lookups are O(1) instead of O(K)
* per cipher row, which would otherwise be O(ciphers Γ— collections) on every change-detection
* cycle.
*/
private manageableCollectionIds = new Set<string>();
Comment thread
JaredScar marked this conversation as resolved.

constructor(
protected cipherAuthorizationService: CipherAuthorizationService,
Expand Down Expand Up @@ -507,12 +514,17 @@ export class VaultItemsComponent<C extends CipherViewLike> {
return this.activeCollection.manage === true;
}

return this.allCollections
.filter((c) => cipher.collectionIds.includes(c.id as any))
.some((collection) => collection.manage);
// Use the pre-computed Set for an O(1) lookup instead of scanning allCollections
// (O(K) per cipher). The set is rebuilt in refreshItems() whenever inputs change.
return cipher.collectionIds.some((id) => this.manageableCollectionIds.has(id as string));
}

private refreshItems() {
// Rebuild the manageable-collection lookup Set so canManageCollection() calls are O(1).
this.manageableCollectionIds = new Set(
(this.allCollections ?? []).filter((c) => c.manage).map((c) => c.id as string),
);

const collections: VaultItem<C>[] = this.collections.map((collection) => ({ collection }));
const ciphers: VaultItem<C>[] = this.ciphers
.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,24 +134,28 @@ export class DefaultCollectionAdminService implements CollectionAdminService {
collections: CollectionResponse[] | CollectionAccessDetailsResponse[],
orgKeys: Record<OrganizationId, OrgKey>,
): Promise<CollectionAdminView[]> {
const promises = collections.map(async (c) => {
if (isCollectionAccessDetailsResponse(c)) {
return CollectionAdminView.fromCollectionAccessDetails(
c,
this.encryptService,
orgKeys[organizationId as OrganizationId],
);
}

return await CollectionAdminView.fromCollectionResponse(
c,
this.encryptService,
orgKeys[organizationId as OrganizationId],
const orgKey = orgKeys[organizationId as OrganizationId];

// Promise.all(8K) floods the microtask queue β€” decrypt in chunks with event-loop
// yields so the browser can paint and handle input between each batch.
const DECRYPT_CHUNK_SIZE = 2_000;
const results: CollectionAdminView[] = [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Porting state to the SDK will give better speed improvements.

for (let i = 0; i < collections.length; i += DECRYPT_CHUNK_SIZE) {
const chunk = collections.slice(i, i + DECRYPT_CHUNK_SIZE);
const decrypted = await Promise.all(
chunk.map(async (c) => {
if (isCollectionAccessDetailsResponse(c)) {
return CollectionAdminView.fromCollectionAccessDetails(c, this.encryptService, orgKey);
}
return await CollectionAdminView.fromCollectionResponse(c, this.encryptService, orgKey);
}),
);
});

const r = await Promise.all(promises);
return r;
results.push(...decrypted);
if (i + DECRYPT_CHUNK_SIZE < collections.length) {
await new Promise<void>((r) => setTimeout(r, 0));
}
}
return results;
}

private async encrypt(
Expand Down
25 changes: 19 additions & 6 deletions libs/common/src/vault/services/cipher.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,9 @@ export class CipherService implements CipherServiceAbstraction {

const [cipherViews] = await this.cipherEncryptionService.decryptManyLegacy(ciphers, userId);

// Sort by locale (matching existing behavior)
// Yield one macrotask before the sort so the browser can paint and respond to input
// (e.g. a sidebar click) before the O(N log N) locale-sort blocks the main thread.
await new Promise<void>((r) => setTimeout(r, 0));
cipherViews.sort(this.getLocaleSortingFunction());

return cipherViews;
Expand Down Expand Up @@ -827,12 +829,23 @@ export class CipherService implements CipherServiceAbstraction {

const ciphers = response.data.map((cr) => new Cipher(new CipherData(cr)));
const key = await this.keyService.getOrgKey(organizationId);
const decCiphers: CipherView[] = await Promise.all(
ciphers.map(async (cipher) => {
return await cipher.decrypt(key);
}),
);

// Promise.all(80K) floods the microtask queue β€” decrypt in chunks with event-loop
// yields so the browser can paint and handle input between each batch.
const DECRYPT_CHUNK_SIZE = 2_000;
const decCiphers: CipherView[] = [];
for (let i = 0; i < ciphers.length; i += DECRYPT_CHUNK_SIZE) {
const chunk = ciphers.slice(i, i + DECRYPT_CHUNK_SIZE);
const decrypted = await Promise.all(chunk.map((c) => c.decrypt(key)));
decCiphers.push(...decrypted);
if (i + DECRYPT_CHUNK_SIZE < ciphers.length) {
await new Promise<void>((r) => setTimeout(r, 0));
}
}

// Yield one macrotask before the sort so the browser can paint and respond to input
// before the O(N log N) locale-sort blocks the main thread.
await new Promise<void>((r) => setTimeout(r, 0));
decCiphers.sort(this.getLocaleSortingFunction());
return decCiphers;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,14 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService {
const successful: CipherView[] = [];
const failed: CipherView[] = [];

// Each SDK decrypt() call resolves synchronously (WASM), so each `await` only
// queues a microtask β€” never yielding to the browser event loop. For large
// org vaults (80 K ciphers) this blocks painting and input for several seconds.
// Yielding via setTimeout(0) every CHUNK_SIZE ciphers converts the flood of
// microtasks into discrete macrotasks the browser event loop can schedule around.
const DECRYPT_CHUNK_SIZE = 2_000;
let processed = 0;

for (const cipher of ciphers) {
try {
const sdkCipherView = await ref.value.vault().ciphers().decrypt(cipher.toSdkCipher());
Expand Down Expand Up @@ -246,6 +254,11 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService {
failedView.decryptionFailure = true;
failed.push(failedView);
}

processed++;
if (processed % DECRYPT_CHUNK_SIZE === 0) {
await new Promise<void>((r) => setTimeout(r, 0));
}
}

return [successful, failed] as [CipherView[], CipherView[]];
Expand Down
Loading
Loading