mirror of
https://github.com/bitwarden/browser
synced 2025-12-17 00:33:44 +00:00
PM-22221: Fix a race condition with cipher creation (#15157)
* PM-22221: Fix a race condition with cipher creation * Mocked ciphers$ in tests * Neater tests --------- Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com>
This commit is contained in:
@@ -5,11 +5,12 @@ import { BehaviorSubject, of } from "rxjs";
|
|||||||
|
|
||||||
import { mockAccountServiceWith } from "../../../../spec";
|
import { mockAccountServiceWith } from "../../../../spec";
|
||||||
import { Account } from "../../../auth/abstractions/account.service";
|
import { Account } from "../../../auth/abstractions/account.service";
|
||||||
import { UserId } from "../../../types/guid";
|
import { CipherId, UserId } from "../../../types/guid";
|
||||||
import { CipherService, EncryptionContext } from "../../../vault/abstractions/cipher.service";
|
import { CipherService, EncryptionContext } from "../../../vault/abstractions/cipher.service";
|
||||||
import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction";
|
import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction";
|
||||||
import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type";
|
import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type";
|
||||||
import { CipherType } from "../../../vault/enums/cipher-type";
|
import { CipherType } from "../../../vault/enums/cipher-type";
|
||||||
|
import { CipherData } from "../../../vault/models/data/cipher.data";
|
||||||
import { Cipher } from "../../../vault/models/domain/cipher";
|
import { Cipher } from "../../../vault/models/domain/cipher";
|
||||||
import { CipherView } from "../../../vault/models/view/cipher.view";
|
import { CipherView } from "../../../vault/models/view/cipher.view";
|
||||||
import { Fido2CredentialView } from "../../../vault/models/view/fido2-credential.view";
|
import { Fido2CredentialView } from "../../../vault/models/view/fido2-credential.view";
|
||||||
@@ -218,9 +219,11 @@ describe("FidoAuthenticatorService", () => {
|
|||||||
beforeEach(async () => {
|
beforeEach(async () => {
|
||||||
existingCipher = createCipherView({ type: CipherType.Login });
|
existingCipher = createCipherView({ type: CipherType.Login });
|
||||||
params = await createParams({ requireResidentKey: false });
|
params = await createParams({ requireResidentKey: false });
|
||||||
cipherService.get.mockImplementation(async (id) =>
|
|
||||||
id === existingCipher.id ? ({ decrypt: () => existingCipher } as any) : undefined,
|
cipherService.ciphers$.mockImplementation((userId: UserId) =>
|
||||||
|
of({ [existingCipher.id as CipherId]: {} as CipherData }),
|
||||||
);
|
);
|
||||||
|
|
||||||
cipherService.getAllDecrypted.mockResolvedValue([existingCipher]);
|
cipherService.getAllDecrypted.mockResolvedValue([existingCipher]);
|
||||||
cipherService.decrypt.mockResolvedValue(existingCipher);
|
cipherService.decrypt.mockResolvedValue(existingCipher);
|
||||||
});
|
});
|
||||||
@@ -351,9 +354,10 @@ describe("FidoAuthenticatorService", () => {
|
|||||||
cipherId,
|
cipherId,
|
||||||
userVerified: false,
|
userVerified: false,
|
||||||
});
|
});
|
||||||
cipherService.get.mockImplementation(async (cipherId) =>
|
cipherService.ciphers$.mockImplementation((userId: UserId) =>
|
||||||
cipherId === cipher.id ? ({ decrypt: () => cipher } as any) : undefined,
|
of({ [cipher.id as CipherId]: {} as CipherData }),
|
||||||
);
|
);
|
||||||
|
|
||||||
cipherService.getAllDecrypted.mockResolvedValue([await cipher]);
|
cipherService.getAllDecrypted.mockResolvedValue([await cipher]);
|
||||||
cipherService.decrypt.mockResolvedValue(cipher);
|
cipherService.decrypt.mockResolvedValue(cipher);
|
||||||
cipherService.encrypt.mockImplementation(async (cipher) => {
|
cipherService.encrypt.mockImplementation(async (cipher) => {
|
||||||
|
|||||||
@@ -1,13 +1,15 @@
|
|||||||
// FIXME: Update this file to be type safe and remove this and next line
|
// FIXME: Update this file to be type safe and remove this and next line
|
||||||
// @ts-strict-ignore
|
// @ts-strict-ignore
|
||||||
import { firstValueFrom } from "rxjs";
|
import { filter, firstValueFrom, map, timeout } from "rxjs";
|
||||||
|
|
||||||
import { AccountService } from "../../../auth/abstractions/account.service";
|
import { AccountService } from "../../../auth/abstractions/account.service";
|
||||||
import { getUserId } from "../../../auth/services/account.service";
|
import { getUserId } from "../../../auth/services/account.service";
|
||||||
|
import { CipherId } from "../../../types/guid";
|
||||||
import { CipherService } from "../../../vault/abstractions/cipher.service";
|
import { CipherService } from "../../../vault/abstractions/cipher.service";
|
||||||
import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction";
|
import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction";
|
||||||
import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type";
|
import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type";
|
||||||
import { CipherType } from "../../../vault/enums/cipher-type";
|
import { CipherType } from "../../../vault/enums/cipher-type";
|
||||||
|
import { Cipher } from "../../../vault/models/domain/cipher";
|
||||||
import { CipherView } from "../../../vault/models/view/cipher.view";
|
import { CipherView } from "../../../vault/models/view/cipher.view";
|
||||||
import { Fido2CredentialView } from "../../../vault/models/view/fido2-credential.view";
|
import { Fido2CredentialView } from "../../../vault/models/view/fido2-credential.view";
|
||||||
import {
|
import {
|
||||||
@@ -149,7 +151,23 @@ export class Fido2AuthenticatorService<ParentWindowReference>
|
|||||||
const activeUserId = await firstValueFrom(
|
const activeUserId = await firstValueFrom(
|
||||||
this.accountService.activeAccount$.pipe(getUserId),
|
this.accountService.activeAccount$.pipe(getUserId),
|
||||||
);
|
);
|
||||||
const encrypted = await this.cipherService.get(cipherId, activeUserId);
|
|
||||||
|
const encrypted = await firstValueFrom(
|
||||||
|
this.cipherService.ciphers$(activeUserId).pipe(
|
||||||
|
map((ciphers) => ciphers[cipherId as CipherId]),
|
||||||
|
filter((c) => c !== undefined),
|
||||||
|
timeout({
|
||||||
|
first: 5000,
|
||||||
|
with: () => {
|
||||||
|
this.logService?.error(
|
||||||
|
`[Fido2Authenticator] Aborting because cipher with ID ${cipherId} could not be found within timeout.`,
|
||||||
|
);
|
||||||
|
throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.Unknown);
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
map((c) => new Cipher(c, null)),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
cipher = await this.cipherService.decrypt(encrypted, activeUserId);
|
cipher = await this.cipherService.decrypt(encrypted, activeUserId);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user