From bef6182243b45029ddd06dd3d94c8f2f50a80555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Thu, 12 Jun 2025 18:53:35 +0200 Subject: [PATCH] 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 --- .../fido2/fido2-authenticator.service.spec.ts | 14 +++++++----- .../fido2/fido2-authenticator.service.ts | 22 +++++++++++++++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts index 78ae8253ee2..fef64399b40 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts @@ -5,11 +5,12 @@ import { BehaviorSubject, of } from "rxjs"; import { mockAccountServiceWith } from "../../../../spec"; 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 { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction"; import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type"; import { CipherType } from "../../../vault/enums/cipher-type"; +import { CipherData } from "../../../vault/models/data/cipher.data"; import { Cipher } from "../../../vault/models/domain/cipher"; import { CipherView } from "../../../vault/models/view/cipher.view"; import { Fido2CredentialView } from "../../../vault/models/view/fido2-credential.view"; @@ -218,9 +219,11 @@ describe("FidoAuthenticatorService", () => { beforeEach(async () => { existingCipher = createCipherView({ type: CipherType.Login }); 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.decrypt.mockResolvedValue(existingCipher); }); @@ -351,9 +354,10 @@ describe("FidoAuthenticatorService", () => { cipherId, userVerified: false, }); - cipherService.get.mockImplementation(async (cipherId) => - cipherId === cipher.id ? ({ decrypt: () => cipher } as any) : undefined, + cipherService.ciphers$.mockImplementation((userId: UserId) => + of({ [cipher.id as CipherId]: {} as CipherData }), ); + cipherService.getAllDecrypted.mockResolvedValue([await cipher]); cipherService.decrypt.mockResolvedValue(cipher); cipherService.encrypt.mockImplementation(async (cipher) => { diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts index a605e466338..bac1b218657 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts @@ -1,13 +1,15 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom } from "rxjs"; +import { filter, firstValueFrom, map, timeout } from "rxjs"; import { AccountService } from "../../../auth/abstractions/account.service"; import { getUserId } from "../../../auth/services/account.service"; +import { CipherId } from "../../../types/guid"; import { CipherService } from "../../../vault/abstractions/cipher.service"; import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction"; import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type"; import { CipherType } from "../../../vault/enums/cipher-type"; +import { Cipher } from "../../../vault/models/domain/cipher"; import { CipherView } from "../../../vault/models/view/cipher.view"; import { Fido2CredentialView } from "../../../vault/models/view/fido2-credential.view"; import { @@ -149,7 +151,23 @@ export class Fido2AuthenticatorService const activeUserId = await firstValueFrom( 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);