mirror of
https://github.com/bitwarden/browser
synced 2025-12-12 14:23:32 +00:00
[PM-26302] Extension when launching a website from a vault item autofill fills wrong cipher (#16810)
* PM-26302 create helper that will assign local data to ciphers and cached ciphers on decryption * remove helper and call local data only on get cipher for url to make operation less expensive * add tests for local data using functions that call the getcipherforurl function * reorder to have early null check
This commit is contained in:
@@ -923,4 +923,75 @@ describe("Cipher Service", () => {
|
|||||||
sub.unsubscribe();
|
sub.unsubscribe();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("getCipherForUrl localData application", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
Object.defineProperty(autofillSettingsService, "autofillOnPageLoadDefault$", {
|
||||||
|
value: of(true),
|
||||||
|
writable: true,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should apply localData to ciphers when getCipherForUrl is called via getLastLaunchedForUrl", async () => {
|
||||||
|
const testUrl = "https://test-url.com";
|
||||||
|
const cipherId = "test-cipher-id" as CipherId;
|
||||||
|
const testLocalData = {
|
||||||
|
lastLaunched: Date.now().valueOf(),
|
||||||
|
lastUsedDate: Date.now().valueOf() - 1000,
|
||||||
|
};
|
||||||
|
|
||||||
|
jest.spyOn(cipherService, "localData$").mockReturnValue(of({ [cipherId]: testLocalData }));
|
||||||
|
|
||||||
|
const mockCipherView = new CipherView();
|
||||||
|
mockCipherView.id = cipherId;
|
||||||
|
mockCipherView.localData = null;
|
||||||
|
|
||||||
|
jest.spyOn(cipherService, "getAllDecryptedForUrl").mockResolvedValue([mockCipherView]);
|
||||||
|
|
||||||
|
const result = await cipherService.getLastLaunchedForUrl(testUrl, userId, true);
|
||||||
|
|
||||||
|
expect(result.localData).toEqual(testLocalData);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should apply localData to ciphers when getCipherForUrl is called via getLastUsedForUrl", async () => {
|
||||||
|
const testUrl = "https://test-url.com";
|
||||||
|
const cipherId = "test-cipher-id" as CipherId;
|
||||||
|
const testLocalData = { lastUsedDate: Date.now().valueOf() - 1000 };
|
||||||
|
|
||||||
|
jest.spyOn(cipherService, "localData$").mockReturnValue(of({ [cipherId]: testLocalData }));
|
||||||
|
|
||||||
|
const mockCipherView = new CipherView();
|
||||||
|
mockCipherView.id = cipherId;
|
||||||
|
mockCipherView.localData = null;
|
||||||
|
|
||||||
|
jest.spyOn(cipherService, "getAllDecryptedForUrl").mockResolvedValue([mockCipherView]);
|
||||||
|
|
||||||
|
const result = await cipherService.getLastUsedForUrl(testUrl, userId, true);
|
||||||
|
|
||||||
|
expect(result.localData).toEqual(testLocalData);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not modify localData if it already matches in getCipherForUrl", async () => {
|
||||||
|
const testUrl = "https://test-url.com";
|
||||||
|
const cipherId = "test-cipher-id" as CipherId;
|
||||||
|
const existingLocalData = {
|
||||||
|
lastLaunched: Date.now().valueOf(),
|
||||||
|
lastUsedDate: Date.now().valueOf() - 1000,
|
||||||
|
};
|
||||||
|
|
||||||
|
jest
|
||||||
|
.spyOn(cipherService, "localData$")
|
||||||
|
.mockReturnValue(of({ [cipherId]: existingLocalData }));
|
||||||
|
|
||||||
|
const mockCipherView = new CipherView();
|
||||||
|
mockCipherView.id = cipherId;
|
||||||
|
mockCipherView.localData = existingLocalData;
|
||||||
|
|
||||||
|
jest.spyOn(cipherService, "getAllDecryptedForUrl").mockResolvedValue([mockCipherView]);
|
||||||
|
|
||||||
|
const result = await cipherService.getLastLaunchedForUrl(testUrl, userId, true);
|
||||||
|
|
||||||
|
expect(result.localData).toBe(existingLocalData);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1947,13 +1947,23 @@ export class CipherService implements CipherServiceAbstraction {
|
|||||||
autofillOnPageLoad: boolean,
|
autofillOnPageLoad: boolean,
|
||||||
): Promise<CipherView> {
|
): Promise<CipherView> {
|
||||||
const cacheKey = autofillOnPageLoad ? "autofillOnPageLoad-" + url : url;
|
const cacheKey = autofillOnPageLoad ? "autofillOnPageLoad-" + url : url;
|
||||||
|
|
||||||
if (!this.sortedCiphersCache.isCached(cacheKey)) {
|
if (!this.sortedCiphersCache.isCached(cacheKey)) {
|
||||||
let ciphers = await this.getAllDecryptedForUrl(url, userId);
|
let ciphers = await this.getAllDecryptedForUrl(url, userId);
|
||||||
if (!ciphers) {
|
|
||||||
|
if (!ciphers?.length) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const localData = await firstValueFrom(this.localData$(userId));
|
||||||
|
if (localData) {
|
||||||
|
for (const view of ciphers) {
|
||||||
|
const data = localData[view.id as CipherId];
|
||||||
|
if (data) {
|
||||||
|
view.localData = data;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (autofillOnPageLoad) {
|
if (autofillOnPageLoad) {
|
||||||
const autofillOnPageLoadDefault = await this.getAutofillOnPageLoadDefault();
|
const autofillOnPageLoadDefault = await this.getAutofillOnPageLoadDefault();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user