1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-07 12:13:45 +00:00

[PM-31668] Race condition in cipher cache clearing causes stale failed decryption state after leaving organization (#18751)

* Refactored the search index to index with the cipherlistview

* Fixed comment

* clear encrypted cipher state to prevent stale emissions during sync
This commit is contained in:
SmithThe4th
2026-02-06 12:10:16 -05:00
committed by GitHub
parent 256fe6305f
commit bf13194b9c
6 changed files with 340 additions and 61 deletions

View File

@@ -182,6 +182,8 @@ 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);

View File

@@ -2,7 +2,6 @@ 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 {
@@ -20,7 +19,7 @@ export abstract class SearchService {
abstract isSearchable(userId: UserId, query: string | null): Promise<boolean>;
abstract indexCiphers(
userId: UserId,
ciphersToIndex: CipherView[],
ciphersToIndex: CipherViewLike[],
indexedEntityGuid?: string,
): Promise<void>;
abstract searchCiphers<C extends CipherViewLike>(

View File

@@ -173,13 +173,14 @@ export class CipherService implements CipherServiceAbstraction {
decryptStartTime = performance.now();
}),
switchMap(async (ciphers) => {
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;
return await this.decryptCiphersWithSdk(ciphers, userId, false);
}),
tap((decrypted) => {
tap(([decrypted, failures]) => {
void Promise.all([
this.setFailedDecryptedCiphers(failures, userId),
this.searchService.indexCiphers(userId, decrypted),
]);
this.logService.measure(
decryptStartTime,
"Vault",
@@ -188,10 +189,11 @@ 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.

View File

@@ -21,7 +21,6 @@ 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.
@@ -169,7 +168,7 @@ export class SearchService implements SearchServiceAbstraction {
async indexCiphers(
userId: UserId,
ciphers: CipherView[],
ciphers: CipherViewLike[],
indexedEntityId?: string,
): Promise<void> {
if (await this.getIsIndexing(userId)) {
@@ -182,34 +181,47 @@ 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: CipherView) => c.id.substr(0, 8) });
builder.field("shortid", {
boost: 100,
extractor: (c: CipherViewLike) => uuidAsString(c.id).substr(0, 8),
});
builder.field("name", {
boost: 10,
});
builder.field("subtitle", {
boost: 5,
extractor: (c: CipherView) => {
if (c.subTitle != null && c.type === CipherType.Card) {
return c.subTitle.replace(/\*/g, "");
extractor: (c: CipherViewLike) => {
const subtitle = CipherViewLikeUtils.subtitle(c);
if (subtitle != null && CipherViewLikeUtils.getType(c) === CipherType.Card) {
return subtitle.replace(/\*/g, "");
}
return c.subTitle;
return subtitle;
},
});
builder.field("notes");
builder.field("notes", { extractor: (c: CipherViewLike) => CipherViewLikeUtils.getNotes(c) });
builder.field("login.username", {
extractor: (c: CipherView) =>
c.type === CipherType.Login && c.login != null ? c.login.username : null,
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),
});
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: CipherView) => this.attachmentExtractor(c, false),
extractor: (c: CipherViewLike) => this.attachmentExtractor(c, false),
});
builder.field("attachments_joined", {
extractor: (c: CipherView) => this.attachmentExtractor(c, true),
extractor: (c: CipherViewLike) => this.attachmentExtractor(c, true),
});
builder.field("organizationid", { extractor: (c: CipherView) => c.organizationId });
builder.field("organizationid", { extractor: (c: CipherViewLike) => c.organizationId });
ciphers = ciphers || [];
ciphers.forEach((c) => builder.add(c));
const index = builder.build();
@@ -400,37 +412,44 @@ export class SearchService implements SearchServiceAbstraction {
return await firstValueFrom(this.searchIsIndexing$(userId));
}
private fieldExtractor(c: CipherView, joined: boolean) {
if (!c.hasFields) {
private fieldExtractor(c: CipherViewLike, joined: boolean) {
const fields = CipherViewLikeUtils.getFields(c);
if (!fields || fields.length === 0) {
return null;
}
let fields: string[] = [];
c.fields.forEach((f) => {
let fieldStrings: string[] = [];
fields.forEach((f) => {
if (f.name != null) {
fields.push(f.name);
fieldStrings.push(f.name);
}
if (f.type === FieldType.Text && f.value != null) {
fields.push(f.value);
// 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);
}
}
});
fields = fields.filter((f) => f.trim() !== "");
if (fields.length === 0) {
fieldStrings = fieldStrings.filter((f) => f.trim() !== "");
if (fieldStrings.length === 0) {
return null;
}
return joined ? fields.join(" ") : fields;
return joined ? fieldStrings.join(" ") : fieldStrings;
}
private attachmentExtractor(c: CipherView, joined: boolean) {
if (!c.hasAttachments) {
private attachmentExtractor(c: CipherViewLike, joined: boolean) {
const attachmentNames = CipherViewLikeUtils.getAttachmentNames(c);
if (!attachmentNames || attachmentNames.length === 0) {
return null;
}
let attachments: string[] = [];
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(".")));
attachmentNames.forEach((fileName) => {
if (fileName != null) {
if (joined && fileName.indexOf(".") > -1) {
attachments.push(fileName.substring(0, fileName.lastIndexOf(".")));
} else {
attachments.push(a.fileName);
attachments.push(fileName);
}
}
});
@@ -441,43 +460,39 @@ export class SearchService implements SearchServiceAbstraction {
return joined ? attachments.join(" ") : attachments;
}
private uriExtractor(c: CipherView) {
if (c.type !== CipherType.Login || c.login == null || !c.login.hasUris) {
private uriExtractor(c: CipherViewLike) {
if (CipherViewLikeUtils.getType(c) !== CipherType.Login) {
return null;
}
const login = CipherViewLikeUtils.getLogin(c);
if (!login?.uris?.length) {
return null;
}
const uris: string[] = [];
c.login.uris.forEach((u) => {
login.uris.forEach((u) => {
if (u.uri == null || u.uri === "") {
return;
}
// Match ports
// Extract port from URI
const portMatch = u.uri.match(/:(\d+)(?:[/?#]|$)/);
const port = portMatch?.[1];
let uri = u.uri;
if (u.hostname !== null) {
uris.push(u.hostname);
const hostname = CipherViewLikeUtils.getUriHostname(u);
if (hostname !== undefined) {
uris.push(hostname);
if (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(`${hostname}:${port}`);
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.substr(protocolIndex + 3);
uri = uri.substring(protocolIndex + 3);
}
const queryIndex = uri.search(/\?|&|#/);
if (queryIndex > -1) {
@@ -486,6 +501,7 @@ export class SearchService implements SearchServiceAbstraction {
}
uris.push(uri);
});
return uris.length > 0 ? uris : null;
}

View File

@@ -651,4 +651,198 @@ 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();
});
});
});
});

View File

@@ -10,6 +10,7 @@ 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";
@@ -290,6 +291,71 @@ 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;
};
}
/**