From 48d18df2854c34f21ab4a4c2cb167ee7ae9baa47 Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Mon, 9 Feb 2026 15:32:49 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"[PM-31668]=20Race=20condition=20in=20?= =?UTF-8?q?cipher=20cache=20clearing=20causes=20stale=20faile=E2=80=A6"=20?= =?UTF-8?q?(#18846)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit bf13194b9cde3dbad47866cfe4fc752935e3e52a. --- .../src/platform/sync/default-sync.service.ts | 2 - .../src/vault/abstractions/search.service.ts | 3 +- .../src/vault/services/cipher.service.ts | 16 +- .../src/vault/services/search.service.ts | 120 +++++------ .../utils/cipher-view-like-utils.spec.ts | 194 ------------------ .../src/vault/utils/cipher-view-like-utils.ts | 66 ------ 6 files changed, 61 insertions(+), 340 deletions(-) diff --git a/libs/common/src/platform/sync/default-sync.service.ts b/libs/common/src/platform/sync/default-sync.service.ts index a25b1b3c210..68c03503e8d 100644 --- a/libs/common/src/platform/sync/default-sync.service.ts +++ b/libs/common/src/platform/sync/default-sync.service.ts @@ -183,8 +183,6 @@ export class DefaultSyncService extends CoreSyncService { const response = await this.inFlightApiCalls.sync; - await this.cipherService.clear(response.profile.id); - await this.syncUserDecryption(response.profile.id, response.userDecryption); await this.syncProfile(response.profile); await this.syncFolders(response.folders, response.profile.id); diff --git a/libs/common/src/vault/abstractions/search.service.ts b/libs/common/src/vault/abstractions/search.service.ts index b4dfc015efe..29575ec3af9 100644 --- a/libs/common/src/vault/abstractions/search.service.ts +++ b/libs/common/src/vault/abstractions/search.service.ts @@ -2,6 +2,7 @@ import { Observable } from "rxjs"; import { SendView } from "../../tools/send/models/view/send.view"; import { IndexedEntityId, UserId } from "../../types/guid"; +import { CipherView } from "../models/view/cipher.view"; import { CipherViewLike } from "../utils/cipher-view-like-utils"; export abstract class SearchService { @@ -19,7 +20,7 @@ export abstract class SearchService { abstract isSearchable(userId: UserId, query: string | null): Promise; abstract indexCiphers( userId: UserId, - ciphersToIndex: CipherViewLike[], + ciphersToIndex: CipherView[], indexedEntityGuid?: string, ): Promise; abstract searchCiphers( diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 696ef49065c..6373a511724 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -173,14 +173,13 @@ export class CipherService implements CipherServiceAbstraction { decryptStartTime = performance.now(); }), switchMap(async (ciphers) => { - return await this.decryptCiphersWithSdk(ciphers, userId, false); + const [decrypted, failures] = await this.decryptCiphersWithSdk(ciphers, userId, false); + void this.setFailedDecryptedCiphers(failures, userId); + // Trigger full decryption and indexing in background + void this.getAllDecrypted(userId); + return decrypted; }), - tap(([decrypted, failures]) => { - void Promise.all([ - this.setFailedDecryptedCiphers(failures, userId), - this.searchService.indexCiphers(userId, decrypted), - ]); - + tap((decrypted) => { this.logService.measure( decryptStartTime, "Vault", @@ -189,11 +188,10 @@ export class CipherService implements CipherServiceAbstraction { [["Items", decrypted.length]], ); }), - map(([decrypted]) => decrypted), ); }), ); - }, this.clearCipherViewsForUser$); + }); /** * Observable that emits an array of decrypted ciphers for the active user. diff --git a/libs/common/src/vault/services/search.service.ts b/libs/common/src/vault/services/search.service.ts index e14a66aad6f..feb6a7494b5 100644 --- a/libs/common/src/vault/services/search.service.ts +++ b/libs/common/src/vault/services/search.service.ts @@ -21,6 +21,7 @@ import { IndexedEntityId, UserId } from "../../types/guid"; import { SearchService as SearchServiceAbstraction } from "../abstractions/search.service"; import { FieldType } from "../enums"; import { CipherType } from "../enums/cipher-type"; +import { CipherView } from "../models/view/cipher.view"; import { CipherViewLike, CipherViewLikeUtils } from "../utils/cipher-view-like-utils"; // Time to wait before performing a search after the user stops typing. @@ -168,7 +169,7 @@ export class SearchService implements SearchServiceAbstraction { async indexCiphers( userId: UserId, - ciphers: CipherViewLike[], + ciphers: CipherView[], indexedEntityId?: string, ): Promise { if (await this.getIsIndexing(userId)) { @@ -181,47 +182,34 @@ export class SearchService implements SearchServiceAbstraction { const builder = new lunr.Builder(); builder.pipeline.add(this.normalizeAccentsPipelineFunction); builder.ref("id"); - builder.field("shortid", { - boost: 100, - extractor: (c: CipherViewLike) => uuidAsString(c.id).substr(0, 8), - }); + builder.field("shortid", { boost: 100, extractor: (c: CipherView) => c.id.substr(0, 8) }); builder.field("name", { boost: 10, }); builder.field("subtitle", { boost: 5, - extractor: (c: CipherViewLike) => { - const subtitle = CipherViewLikeUtils.subtitle(c); - if (subtitle != null && CipherViewLikeUtils.getType(c) === CipherType.Card) { - return subtitle.replace(/\*/g, ""); + extractor: (c: CipherView) => { + if (c.subTitle != null && c.type === CipherType.Card) { + return c.subTitle.replace(/\*/g, ""); } - return subtitle; + return c.subTitle; }, }); - builder.field("notes", { extractor: (c: CipherViewLike) => CipherViewLikeUtils.getNotes(c) }); + builder.field("notes"); builder.field("login.username", { - extractor: (c: CipherViewLike) => { - const login = CipherViewLikeUtils.getLogin(c); - return login?.username ?? null; - }, - }); - builder.field("login.uris", { - boost: 2, - extractor: (c: CipherViewLike) => this.uriExtractor(c), - }); - builder.field("fields", { - extractor: (c: CipherViewLike) => this.fieldExtractor(c, false), - }); - builder.field("fields_joined", { - extractor: (c: CipherViewLike) => this.fieldExtractor(c, true), + extractor: (c: CipherView) => + c.type === CipherType.Login && c.login != null ? c.login.username : null, }); + builder.field("login.uris", { boost: 2, extractor: (c: CipherView) => this.uriExtractor(c) }); + builder.field("fields", { extractor: (c: CipherView) => this.fieldExtractor(c, false) }); + builder.field("fields_joined", { extractor: (c: CipherView) => this.fieldExtractor(c, true) }); builder.field("attachments", { - extractor: (c: CipherViewLike) => this.attachmentExtractor(c, false), + extractor: (c: CipherView) => this.attachmentExtractor(c, false), }); builder.field("attachments_joined", { - extractor: (c: CipherViewLike) => this.attachmentExtractor(c, true), + extractor: (c: CipherView) => this.attachmentExtractor(c, true), }); - builder.field("organizationid", { extractor: (c: CipherViewLike) => c.organizationId }); + builder.field("organizationid", { extractor: (c: CipherView) => c.organizationId }); ciphers = ciphers || []; ciphers.forEach((c) => builder.add(c)); const index = builder.build(); @@ -412,44 +400,37 @@ export class SearchService implements SearchServiceAbstraction { return await firstValueFrom(this.searchIsIndexing$(userId)); } - private fieldExtractor(c: CipherViewLike, joined: boolean) { - const fields = CipherViewLikeUtils.getFields(c); - if (!fields || fields.length === 0) { + private fieldExtractor(c: CipherView, joined: boolean) { + if (!c.hasFields) { return null; } - let fieldStrings: string[] = []; - fields.forEach((f) => { + let fields: string[] = []; + c.fields.forEach((f) => { if (f.name != null) { - fieldStrings.push(f.name); + fields.push(f.name); } - // For CipherListView, value is only populated for Text fields - // For CipherView, we check the type explicitly - if (f.value != null) { - const fieldType = (f as { type?: FieldType }).type; - if (fieldType === undefined || fieldType === FieldType.Text) { - fieldStrings.push(f.value); - } + if (f.type === FieldType.Text && f.value != null) { + fields.push(f.value); } }); - fieldStrings = fieldStrings.filter((f) => f.trim() !== ""); - if (fieldStrings.length === 0) { + fields = fields.filter((f) => f.trim() !== ""); + if (fields.length === 0) { return null; } - return joined ? fieldStrings.join(" ") : fieldStrings; + return joined ? fields.join(" ") : fields; } - private attachmentExtractor(c: CipherViewLike, joined: boolean) { - const attachmentNames = CipherViewLikeUtils.getAttachmentNames(c); - if (!attachmentNames || attachmentNames.length === 0) { + private attachmentExtractor(c: CipherView, joined: boolean) { + if (!c.hasAttachments) { return null; } let attachments: string[] = []; - attachmentNames.forEach((fileName) => { - if (fileName != null) { - if (joined && fileName.indexOf(".") > -1) { - attachments.push(fileName.substring(0, fileName.lastIndexOf("."))); + c.attachments.forEach((a) => { + if (a != null && a.fileName != null) { + if (joined && a.fileName.indexOf(".") > -1) { + attachments.push(a.fileName.substr(0, a.fileName.lastIndexOf("."))); } else { - attachments.push(fileName); + attachments.push(a.fileName); } } }); @@ -460,39 +441,43 @@ export class SearchService implements SearchServiceAbstraction { return joined ? attachments.join(" ") : attachments; } - private uriExtractor(c: CipherViewLike) { - if (CipherViewLikeUtils.getType(c) !== CipherType.Login) { - return null; - } - const login = CipherViewLikeUtils.getLogin(c); - if (!login?.uris?.length) { + private uriExtractor(c: CipherView) { + if (c.type !== CipherType.Login || c.login == null || !c.login.hasUris) { return null; } const uris: string[] = []; - login.uris.forEach((u) => { + c.login.uris.forEach((u) => { if (u.uri == null || u.uri === "") { return; } - // Extract port from URI + // Match ports const portMatch = u.uri.match(/:(\d+)(?:[/?#]|$)/); const port = portMatch?.[1]; - const hostname = CipherViewLikeUtils.getUriHostname(u); - if (hostname !== undefined) { - uris.push(hostname); + let uri = u.uri; + + if (u.hostname !== null) { + uris.push(u.hostname); if (port) { - uris.push(`${hostname}:${port}`); + uris.push(`${u.hostname}:${port}`); + uris.push(port); + } + return; + } else { + const slash = uri.indexOf("/"); + const hostPart = slash > -1 ? uri.substring(0, slash) : uri; + uris.push(hostPart); + if (port) { + uris.push(`${hostPart}`); uris.push(port); } } - // Add processed URI (strip protocol and query params for non-regex matches) - let uri = u.uri; if (u.match !== UriMatchStrategy.RegularExpression) { const protocolIndex = uri.indexOf("://"); if (protocolIndex > -1) { - uri = uri.substring(protocolIndex + 3); + uri = uri.substr(protocolIndex + 3); } const queryIndex = uri.search(/\?|&|#/); if (queryIndex > -1) { @@ -501,7 +486,6 @@ export class SearchService implements SearchServiceAbstraction { } uris.push(uri); }); - return uris.length > 0 ? uris : null; } diff --git a/libs/common/src/vault/utils/cipher-view-like-utils.spec.ts b/libs/common/src/vault/utils/cipher-view-like-utils.spec.ts index 2a7bfac2970..56b94fcf3ce 100644 --- a/libs/common/src/vault/utils/cipher-view-like-utils.spec.ts +++ b/libs/common/src/vault/utils/cipher-view-like-utils.spec.ts @@ -651,198 +651,4 @@ describe("CipherViewLikeUtils", () => { expect(CipherViewLikeUtils.decryptionFailure(cipherListView)).toBe(false); }); }); - - describe("getNotes", () => { - describe("CipherView", () => { - it("returns notes when present", () => { - const cipherView = createCipherView(); - cipherView.notes = "This is a test note"; - - expect(CipherViewLikeUtils.getNotes(cipherView)).toBe("This is a test note"); - }); - - it("returns undefined when notes are not present", () => { - const cipherView = createCipherView(); - cipherView.notes = undefined; - - expect(CipherViewLikeUtils.getNotes(cipherView)).toBeUndefined(); - }); - }); - - describe("CipherListView", () => { - it("returns notes when present", () => { - const cipherListView = { - type: "secureNote", - notes: "List view notes", - } as CipherListView; - - expect(CipherViewLikeUtils.getNotes(cipherListView)).toBe("List view notes"); - }); - - it("returns undefined when notes are not present", () => { - const cipherListView = { - type: "secureNote", - } as CipherListView; - - expect(CipherViewLikeUtils.getNotes(cipherListView)).toBeUndefined(); - }); - }); - }); - - describe("getFields", () => { - describe("CipherView", () => { - it("returns fields when present", () => { - const cipherView = createCipherView(); - cipherView.fields = [ - { name: "Field1", value: "Value1" } as any, - { name: "Field2", value: "Value2" } as any, - ]; - - const fields = CipherViewLikeUtils.getFields(cipherView); - - expect(fields).toHaveLength(2); - expect(fields?.[0].name).toBe("Field1"); - expect(fields?.[0].value).toBe("Value1"); - expect(fields?.[1].name).toBe("Field2"); - expect(fields?.[1].value).toBe("Value2"); - }); - - it("returns empty array when fields array is empty", () => { - const cipherView = createCipherView(); - cipherView.fields = []; - - expect(CipherViewLikeUtils.getFields(cipherView)).toEqual([]); - }); - }); - - describe("CipherListView", () => { - it("returns fields when present", () => { - const cipherListView = { - type: { login: {} }, - fields: [ - { name: "Username", value: "user@example.com" }, - { name: "API Key", value: "abc123" }, - ], - } as CipherListView; - - const fields = CipherViewLikeUtils.getFields(cipherListView); - - expect(fields).toHaveLength(2); - expect(fields?.[0].name).toBe("Username"); - expect(fields?.[0].value).toBe("user@example.com"); - expect(fields?.[1].name).toBe("API Key"); - expect(fields?.[1].value).toBe("abc123"); - }); - - it("returns empty array when fields array is empty", () => { - const cipherListView = { - type: "secureNote", - fields: [], - } as unknown as CipherListView; - - expect(CipherViewLikeUtils.getFields(cipherListView)).toEqual([]); - }); - - it("returns undefined when fields are not present", () => { - const cipherListView = { - type: "secureNote", - } as CipherListView; - - expect(CipherViewLikeUtils.getFields(cipherListView)).toBeUndefined(); - }); - }); - }); - - describe("getAttachmentNames", () => { - describe("CipherView", () => { - it("returns attachment filenames when present", () => { - const cipherView = createCipherView(); - const attachment1 = new AttachmentView(); - attachment1.id = "1"; - attachment1.fileName = "document.pdf"; - const attachment2 = new AttachmentView(); - attachment2.id = "2"; - attachment2.fileName = "image.png"; - const attachment3 = new AttachmentView(); - attachment3.id = "3"; - attachment3.fileName = "spreadsheet.xlsx"; - cipherView.attachments = [attachment1, attachment2, attachment3]; - - const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherView); - - expect(attachmentNames).toEqual(["document.pdf", "image.png", "spreadsheet.xlsx"]); - }); - - it("filters out null and undefined filenames", () => { - const cipherView = createCipherView(); - const attachment1 = new AttachmentView(); - attachment1.id = "1"; - attachment1.fileName = "valid.pdf"; - const attachment2 = new AttachmentView(); - attachment2.id = "2"; - attachment2.fileName = null as any; - const attachment3 = new AttachmentView(); - attachment3.id = "3"; - attachment3.fileName = undefined; - const attachment4 = new AttachmentView(); - attachment4.id = "4"; - attachment4.fileName = "another.txt"; - cipherView.attachments = [attachment1, attachment2, attachment3, attachment4]; - - const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherView); - - expect(attachmentNames).toEqual(["valid.pdf", "another.txt"]); - }); - - it("returns empty array when attachments have no filenames", () => { - const cipherView = createCipherView(); - const attachment1 = new AttachmentView(); - attachment1.id = "1"; - const attachment2 = new AttachmentView(); - attachment2.id = "2"; - cipherView.attachments = [attachment1, attachment2]; - - const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherView); - - expect(attachmentNames).toEqual([]); - }); - - it("returns empty array for empty attachments array", () => { - const cipherView = createCipherView(); - cipherView.attachments = []; - - expect(CipherViewLikeUtils.getAttachmentNames(cipherView)).toEqual([]); - }); - }); - - describe("CipherListView", () => { - it("returns attachment names when present", () => { - const cipherListView = { - type: "secureNote", - attachmentNames: ["report.pdf", "photo.jpg", "data.csv"], - } as CipherListView; - - const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherListView); - - expect(attachmentNames).toEqual(["report.pdf", "photo.jpg", "data.csv"]); - }); - - it("returns empty array when attachmentNames is empty", () => { - const cipherListView = { - type: "secureNote", - attachmentNames: [], - } as unknown as CipherListView; - - expect(CipherViewLikeUtils.getAttachmentNames(cipherListView)).toEqual([]); - }); - - it("returns undefined when attachmentNames is not present", () => { - const cipherListView = { - type: "secureNote", - } as CipherListView; - - expect(CipherViewLikeUtils.getAttachmentNames(cipherListView)).toBeUndefined(); - }); - }); - }); }); diff --git a/libs/common/src/vault/utils/cipher-view-like-utils.ts b/libs/common/src/vault/utils/cipher-view-like-utils.ts index 5359bfb958f..04adb8d4832 100644 --- a/libs/common/src/vault/utils/cipher-view-like-utils.ts +++ b/libs/common/src/vault/utils/cipher-view-like-utils.ts @@ -10,7 +10,6 @@ import { LoginUriView as LoginListUriView, } from "@bitwarden/sdk-internal"; -import { Utils } from "../../platform/misc/utils"; import { CipherType } from "../enums"; import { Cipher } from "../models/domain/cipher"; import { CardView } from "../models/view/card.view"; @@ -291,71 +290,6 @@ export class CipherViewLikeUtils { static decryptionFailure = (cipher: CipherViewLike): boolean => { return "decryptionFailure" in cipher ? cipher.decryptionFailure : false; }; - - /** - * Returns the notes from the cipher. - * - * @param cipher - The cipher to extract notes from (either `CipherView` or `CipherListView`) - * @returns The notes string if present, or `undefined` if not set - */ - static getNotes = (cipher: CipherViewLike): string | undefined => { - return cipher.notes; - }; - - /** - * Returns the fields from the cipher. - * - * @param cipher - The cipher to extract fields from (either `CipherView` or `CipherListView`) - * @returns Array of field objects with `name` and `value` properties, `undefined` if not set - */ - static getFields = ( - cipher: CipherViewLike, - ): { name?: string | null; value?: string | undefined }[] | undefined => { - if (this.isCipherListView(cipher)) { - return cipher.fields; - } - return cipher.fields; - }; - - /** - * Returns attachment filenames from the cipher. - * - * @param cipher - The cipher to extract attachment names from (either `CipherView` or `CipherListView`) - * @returns Array of attachment filenames, `undefined` if attachments are not present - */ - static getAttachmentNames = (cipher: CipherViewLike): string[] | undefined => { - if (this.isCipherListView(cipher)) { - return cipher.attachmentNames; - } - - return cipher.attachments - ?.map((a) => a.fileName) - .filter((name): name is string => name != null); - }; - - /** - * Extracts hostname from a login URI. - * - * @param uri - The URI object (either `LoginUriView` class or `LoginListUriView`) - * @returns The hostname if available, `undefined` otherwise - * - * @remarks - * - For `LoginUriView` (CipherView): Uses the built-in `hostname` getter - * - For `LoginListUriView` (CipherListView): Computes hostname using `Utils.getHostname()` - * - Returns `undefined` for RegularExpression match types or when hostname cannot be extracted - */ - static getUriHostname = (uri: LoginListUriView | LoginUriView): string | undefined => { - if ("hostname" in uri && typeof uri.hostname !== "undefined") { - return uri.hostname ?? undefined; - } - - if (uri.match !== UriMatchStrategy.RegularExpression && uri.uri) { - const hostname = Utils.getHostname(uri.uri); - return hostname === "" ? undefined : hostname; - } - - return undefined; - }; } /**