From d4822b1083e519f847818ec06a63216786f72540 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 12 Sep 2023 15:32:02 +0200 Subject: [PATCH] [PM-3807] Store all passkeys as login cipher type (#6255) * [PM-3807] feat: add `discoverable` property to fido2keys * [PM-3807] feat: assign discoverable property during creation * [PM-3807] feat: save discoverable field to server * [PM-3807] feat: filter credentials by rpId AND discoverable * [PM-3807] chore: remove discoverable tests which are no longer needed * [PM-3807] chore: remove all logic for handling standalone Fido2Key View and components will be cleaned up as part of UI tickets * [PM-3807] fix: add missing discoverable property handling to tests --- ...fido2-authenticator.service.abstraction.ts | 2 +- libs/common/src/vault/api/fido2-key.api.ts | 2 + .../src/vault/models/data/cipher.data.ts | 7 - .../src/vault/models/data/fido2-key.data.ts | 2 + libs/common/src/vault/models/domain/cipher.ts | 14 -- .../src/vault/models/domain/fido2-key.spec.ts | 9 +- .../src/vault/models/domain/fido2-key.ts | 16 +++ .../src/vault/models/domain/login.spec.ts | 2 + .../vault/models/request/cipher.request.ts | 38 +----- .../vault/models/response/cipher.response.ts | 7 - .../src/vault/models/view/cipher.view.ts | 5 - .../src/vault/models/view/fido2-key.view.ts | 1 + .../src/vault/services/cipher.service.ts | 28 +--- .../fido2/fido2-authenticator.service.spec.ts | 121 ++++++------------ .../fido2/fido2-authenticator.service.ts | 13 +- 15 files changed, 84 insertions(+), 183 deletions(-) diff --git a/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts index 3e842700bc3..521c332344c 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts @@ -81,7 +81,7 @@ export interface Fido2AuthenticatorMakeCredentialsParams { }; /** A Boolean value that indicates that individually-identifying attestation MAY be returned by the authenticator. */ enterpriseAttestationPossible?: boolean; // Ignored by bitwarden at the moment - /** The effective resident key requirement for credential creation, a Boolean value determined by the client. */ + /** The effective resident key requirement for credential creation, a Boolean value determined by the client. Resident is synonymous with discoverable. */ requireResidentKey: boolean; requireUserVerification: boolean; /** Forwarded to user interface */ diff --git a/libs/common/src/vault/api/fido2-key.api.ts b/libs/common/src/vault/api/fido2-key.api.ts index c98a06f1d15..0d3f425bd94 100644 --- a/libs/common/src/vault/api/fido2-key.api.ts +++ b/libs/common/src/vault/api/fido2-key.api.ts @@ -11,6 +11,7 @@ export class Fido2KeyApi extends BaseResponse { counter: string; rpName: string; userDisplayName: string; + discoverable: string; constructor(data: any = null) { super(data); @@ -28,5 +29,6 @@ export class Fido2KeyApi extends BaseResponse { this.counter = this.getResponseProperty("Counter"); this.rpName = this.getResponseProperty("RpName"); this.userDisplayName = this.getResponseProperty("UserDisplayName"); + this.discoverable = this.getResponseProperty("Discoverable"); } } diff --git a/libs/common/src/vault/models/data/cipher.data.ts b/libs/common/src/vault/models/data/cipher.data.ts index 1c995b32aea..2f83ee194b4 100644 --- a/libs/common/src/vault/models/data/cipher.data.ts +++ b/libs/common/src/vault/models/data/cipher.data.ts @@ -4,7 +4,6 @@ import { CipherResponse } from "../response/cipher.response"; import { AttachmentData } from "./attachment.data"; import { CardData } from "./card.data"; -import { Fido2KeyData } from "./fido2-key.data"; import { FieldData } from "./field.data"; import { IdentityData } from "./identity.data"; import { LoginData } from "./login.data"; @@ -27,7 +26,6 @@ export class CipherData { secureNote?: SecureNoteData; card?: CardData; identity?: IdentityData; - fido2Key?: Fido2KeyData; fields?: FieldData[]; attachments?: AttachmentData[]; passwordHistory?: PasswordHistoryData[]; @@ -60,8 +58,6 @@ export class CipherData { switch (this.type) { case CipherType.Login: this.login = new LoginData(response.login); - this.fido2Key = - response.fido2Key != undefined ? new Fido2KeyData(response.fido2Key) : undefined; break; case CipherType.SecureNote: this.secureNote = new SecureNoteData(response.secureNote); @@ -72,9 +68,6 @@ export class CipherData { case CipherType.Identity: this.identity = new IdentityData(response.identity); break; - case CipherType.Fido2Key: - this.fido2Key = new Fido2KeyData(response.fido2Key); - break; default: break; } diff --git a/libs/common/src/vault/models/data/fido2-key.data.ts b/libs/common/src/vault/models/data/fido2-key.data.ts index 45d2feadf4f..6f0c49f3e8d 100644 --- a/libs/common/src/vault/models/data/fido2-key.data.ts +++ b/libs/common/src/vault/models/data/fido2-key.data.ts @@ -11,6 +11,7 @@ export class Fido2KeyData { counter: string; rpName: string; userDisplayName: string; + discoverable: string; constructor(data?: Fido2KeyApi) { if (data == null) { @@ -27,5 +28,6 @@ export class Fido2KeyData { this.counter = data.counter; this.rpName = data.rpName; this.userDisplayName = data.userDisplayName; + this.discoverable = data.discoverable; } } diff --git a/libs/common/src/vault/models/domain/cipher.ts b/libs/common/src/vault/models/domain/cipher.ts index ec64d40ee55..f85b6cb45c9 100644 --- a/libs/common/src/vault/models/domain/cipher.ts +++ b/libs/common/src/vault/models/domain/cipher.ts @@ -13,7 +13,6 @@ import { CipherView } from "../view/cipher.view"; import { Attachment } from "./attachment"; import { Card } from "./card"; -import { Fido2Key } from "./fido2-key"; import { Field } from "./field"; import { Identity } from "./identity"; import { Login } from "./login"; @@ -39,7 +38,6 @@ export class Cipher extends Domain implements Decryptable { identity: Identity; card: Card; secureNote: SecureNote; - fido2Key: Fido2Key; attachments: Attachment[]; fields: Field[]; passwordHistory: Password[]; @@ -96,9 +94,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.Identity: this.identity = new Identity(obj.identity); break; - case CipherType.Fido2Key: - this.fido2Key = new Fido2Key(obj.fido2Key); - break; default: break; } @@ -148,9 +143,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.Identity: model.identity = await this.identity.decrypt(this.organizationId, encKey); break; - case CipherType.Fido2Key: - model.fido2Key = await this.fido2Key.decrypt(this.organizationId, encKey); - break; default: break; } @@ -236,9 +228,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.Identity: c.identity = this.identity.toIdentityData(); break; - case CipherType.Fido2Key: - c.fido2Key = this.fido2Key.toFido2KeyData(); - break; default: break; } @@ -292,9 +281,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.SecureNote: domain.secureNote = SecureNote.fromJSON(obj.secureNote); break; - case CipherType.Fido2Key: - domain.fido2Key = Fido2Key.fromJSON(obj.fido2Key); - break; default: break; } diff --git a/libs/common/src/vault/models/domain/fido2-key.spec.ts b/libs/common/src/vault/models/domain/fido2-key.spec.ts index 6804963fbe1..365dbb1c6e1 100644 --- a/libs/common/src/vault/models/domain/fido2-key.spec.ts +++ b/libs/common/src/vault/models/domain/fido2-key.spec.ts @@ -22,6 +22,7 @@ describe("Fido2Key", () => { rpName: null, userDisplayName: null, counter: null, + discoverable: null, }); }); @@ -37,6 +38,7 @@ describe("Fido2Key", () => { counter: "counter", rpName: "rpName", userDisplayName: "userDisplayName", + discoverable: "discoverable", }; const fido2Key = new Fido2Key(data); @@ -51,6 +53,7 @@ describe("Fido2Key", () => { counter: { encryptedString: "counter", encryptionType: 0 }, rpName: { encryptedString: "rpName", encryptionType: 0 }, userDisplayName: { encryptedString: "userDisplayName", encryptionType: 0 }, + discoverable: { encryptedString: "discoverable", encryptionType: 0 }, }); }); @@ -76,6 +79,7 @@ describe("Fido2Key", () => { fido2Key.counter = mockEnc("2"); fido2Key.rpName = mockEnc("rpName"); fido2Key.userDisplayName = mockEnc("userDisplayName"); + fido2Key.discoverable = mockEnc("true"); const fido2KeyView = await fido2Key.decrypt(null); @@ -90,6 +94,7 @@ describe("Fido2Key", () => { rpName: "rpName", userDisplayName: "userDisplayName", counter: 2, + discoverable: true, }); }); }); @@ -104,9 +109,10 @@ describe("Fido2Key", () => { keyValue: "keyValue", rpId: "rpId", userHandle: "userHandle", - counter: "counter", + counter: "2", rpName: "rpName", userDisplayName: "userDisplayName", + discoverable: "true", }; const fido2Key = new Fido2Key(data); @@ -129,6 +135,7 @@ describe("Fido2Key", () => { fido2Key.counter = createEncryptedEncString("2"); fido2Key.rpName = createEncryptedEncString("rpName"); fido2Key.userDisplayName = createEncryptedEncString("userDisplayName"); + fido2Key.discoverable = createEncryptedEncString("discoverable"); const json = JSON.stringify(fido2Key); const result = Fido2Key.fromJSON(JSON.parse(json)); diff --git a/libs/common/src/vault/models/domain/fido2-key.ts b/libs/common/src/vault/models/domain/fido2-key.ts index e03920f3d68..61abd655ee2 100644 --- a/libs/common/src/vault/models/domain/fido2-key.ts +++ b/libs/common/src/vault/models/domain/fido2-key.ts @@ -17,6 +17,7 @@ export class Fido2Key extends Domain { counter: EncString; rpName: EncString; userDisplayName: EncString; + discoverable: EncString; constructor(obj?: Fido2KeyData) { super(); @@ -38,6 +39,7 @@ export class Fido2Key extends Domain { counter: null, rpName: null, userDisplayName: null, + discoverable: null, }, [] ); @@ -56,6 +58,7 @@ export class Fido2Key extends Domain { userHandle: null, rpName: null, userDisplayName: null, + discoverable: null, }, orgId, encKey @@ -72,6 +75,16 @@ export class Fido2Key extends Domain { // Counter will end up as NaN if this fails view.counter = parseInt(counter); + const { discoverable } = await this.decryptObj( + { discoverable: "" }, + { + discoverable: null, + }, + orgId, + encKey + ); + view.discoverable = discoverable === "true"; + return view; } @@ -88,6 +101,7 @@ export class Fido2Key extends Domain { counter: null, rpName: null, userDisplayName: null, + discoverable: null, }); return i; } @@ -107,6 +121,7 @@ export class Fido2Key extends Domain { const counter = EncString.fromJSON(obj.counter); const rpName = EncString.fromJSON(obj.rpName); const userDisplayName = EncString.fromJSON(obj.userDisplayName); + const discoverable = EncString.fromJSON(obj.discoverable); return Object.assign(new Fido2Key(), obj, { credentialId, @@ -119,6 +134,7 @@ export class Fido2Key extends Domain { counter, rpName, userDisplayName, + discoverable, }); } } diff --git a/libs/common/src/vault/models/domain/login.spec.ts b/libs/common/src/vault/models/domain/login.spec.ts index 30b3077cb32..f2ec3dbf965 100644 --- a/libs/common/src/vault/models/domain/login.spec.ts +++ b/libs/common/src/vault/models/domain/login.spec.ts @@ -123,6 +123,7 @@ describe("Login DTO", () => { counter: "counter" as EncryptedString, rpName: "rpName" as EncryptedString, userDisplayName: "userDisplayName" as EncryptedString, + discoverable: "discoverable" as EncryptedString, }, }); @@ -143,6 +144,7 @@ describe("Login DTO", () => { counter: "counter_fromJSON", rpName: "rpName_fromJSON", userDisplayName: "userDisplayName_fromJSON", + discoverable: "discoverable_fromJSON", }, }); expect(actual).toBeInstanceOf(Login); diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts index 5ca6cfc7817..8c27ec597b5 100644 --- a/libs/common/src/vault/models/request/cipher.request.ts +++ b/libs/common/src/vault/models/request/cipher.request.ts @@ -23,7 +23,6 @@ export class CipherRequest { secureNote: SecureNoteApi; card: CardApi; identity: IdentityApi; - fido2Key: Fido2KeyApi; fields: FieldApi[]; passwordHistory: PasswordHistoryRequest[]; // Deprecated, remove at some point and rename attachments2 to attachments @@ -104,6 +103,10 @@ export class CipherRequest { cipher.login.fido2Key.userDisplayName != null ? cipher.login.fido2Key.userDisplayName.encryptedString : null; + this.login.fido2Key.discoverable = + cipher.login.fido2Key.discoverable != null + ? cipher.login.fido2Key.discoverable.encryptedString + : null; } break; case CipherType.SecureNote: @@ -165,39 +168,6 @@ export class CipherRequest { ? cipher.identity.licenseNumber.encryptedString : null; break; - case CipherType.Fido2Key: - this.fido2Key = new Fido2KeyApi(); - this.fido2Key.credentialId = - cipher.fido2Key.credentialId != null - ? cipher.fido2Key.credentialId.encryptedString - : null; - this.fido2Key.keyType = - cipher.fido2Key.keyType != null - ? (cipher.fido2Key.keyType.encryptedString as "public-key") - : null; - this.fido2Key.keyAlgorithm = - cipher.fido2Key.keyAlgorithm != null - ? (cipher.fido2Key.keyAlgorithm.encryptedString as "ECDSA") - : null; - this.fido2Key.keyCurve = - cipher.fido2Key.keyCurve != null - ? (cipher.fido2Key.keyCurve.encryptedString as "P-256") - : null; - this.fido2Key.keyValue = - cipher.fido2Key.keyValue != null ? cipher.fido2Key.keyValue.encryptedString : null; - this.fido2Key.rpId = - cipher.fido2Key.rpId != null ? cipher.fido2Key.rpId.encryptedString : null; - this.fido2Key.rpName = - cipher.fido2Key.rpName != null ? cipher.fido2Key.rpName.encryptedString : null; - this.fido2Key.counter = - cipher.fido2Key.counter != null ? cipher.fido2Key.counter.encryptedString : null; - this.fido2Key.userHandle = - cipher.fido2Key.userHandle != null ? cipher.fido2Key.userHandle.encryptedString : null; - this.fido2Key.userDisplayName = - cipher.fido2Key.userDisplayName != null - ? cipher.fido2Key.userDisplayName.encryptedString - : null; - break; default: break; } diff --git a/libs/common/src/vault/models/response/cipher.response.ts b/libs/common/src/vault/models/response/cipher.response.ts index 52d35708a8f..71e43373775 100644 --- a/libs/common/src/vault/models/response/cipher.response.ts +++ b/libs/common/src/vault/models/response/cipher.response.ts @@ -4,7 +4,6 @@ import { IdentityApi } from "../../../models/api/identity.api"; import { LoginApi } from "../../../models/api/login.api"; import { SecureNoteApi } from "../../../models/api/secure-note.api"; import { BaseResponse } from "../../../models/response/base.response"; -import { Fido2KeyApi } from "../../api/fido2-key.api"; import { CipherRepromptType } from "../../enums/cipher-reprompt-type"; import { AttachmentResponse } from "./attachment.response"; @@ -22,7 +21,6 @@ export class CipherResponse extends BaseResponse { card: CardApi; identity: IdentityApi; secureNote: SecureNoteApi; - fido2Key: Fido2KeyApi; favorite: boolean; edit: boolean; viewPassword: boolean; @@ -76,11 +74,6 @@ export class CipherResponse extends BaseResponse { this.secureNote = new SecureNoteApi(secureNote); } - const fido2Key = this.getResponseProperty("Fido2Key"); - if (fido2Key != null) { - this.fido2Key = new Fido2KeyApi(fido2Key); - } - const fields = this.getResponseProperty("Fields"); if (fields != null) { this.fields = fields.map((f: any) => new FieldApi(f)); diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 608ce4571e6..4b342fd57fb 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -78,8 +78,6 @@ export class CipherView implements View, InitializerMetadata { return this.card; case CipherType.Identity: return this.identity; - case CipherType.Fido2Key: - return this.fido2Key; default: break; } @@ -174,9 +172,6 @@ export class CipherView implements View, InitializerMetadata { case CipherType.SecureNote: view.secureNote = SecureNoteView.fromJSON(obj.secureNote); break; - case CipherType.Fido2Key: - view.fido2Key = Fido2KeyView.fromJSON(obj.fido2Key); - break; default: break; } diff --git a/libs/common/src/vault/models/view/fido2-key.view.ts b/libs/common/src/vault/models/view/fido2-key.view.ts index 6fec5dbf25d..8af29af70d3 100644 --- a/libs/common/src/vault/models/view/fido2-key.view.ts +++ b/libs/common/src/vault/models/view/fido2-key.view.ts @@ -13,6 +13,7 @@ export class Fido2KeyView extends ItemView { counter: number; rpName: string; userDisplayName: string; + discoverable: boolean; get subTitle(): string { return this.userDisplayName; diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 66479f561ad..e1801c47899 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1112,6 +1112,10 @@ export class CipherService implements CipherServiceAbstraction { String(model.login.fido2Key.counter), key ); + cipher.login.fido2Key.discoverable = await this.cryptoService.encrypt( + String(model.login.fido2Key.discoverable), + key + ); } return; case CipherType.SecureNote: @@ -1162,30 +1166,6 @@ export class CipherService implements CipherServiceAbstraction { key ); return; - case CipherType.Fido2Key: - cipher.fido2Key = new Fido2Key(); - await this.encryptObjProperty( - model.fido2Key, - cipher.fido2Key, - { - credentialId: null, - keyType: null, - keyAlgorithm: null, - keyCurve: null, - keyValue: null, - rpId: null, - rpName: null, - userHandle: null, - userDisplayName: null, - origin: null, - }, - key - ); - cipher.fido2Key.counter = await this.cryptoService.encrypt( - String(model.fido2Key.counter), - key - ); - break; default: throw new Error("Unknown cipher type."); } diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index 74e674f9c55..c7fb3f53244 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -101,7 +101,7 @@ describe("FidoAuthenticatorService", () => { describe.skip("when extensions parameter is present", () => undefined); - describe("vault contains excluded non-discoverable credential", () => { + describe("vault contains excluded credential", () => { let excludedCipher: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; @@ -179,90 +179,13 @@ describe("FidoAuthenticatorService", () => { ); }); - describe("vault contains excluded discoverable credential", () => { - let excludedCipherView: CipherView; - let params: Fido2AuthenticatorMakeCredentialsParams; - - beforeEach(async () => { - excludedCipherView = createCipherView(); - params = await createParams({ - excludeCredentialDescriptorList: [ - { - id: guidToRawFormat(excludedCipherView.fido2Key.credentialId), - type: "public-key", - }, - ], - }); - cipherService.get.mockImplementation(async (id) => - id === excludedCipherView.id - ? ({ decrypt: async () => excludedCipherView } as any) - : undefined - ); - cipherService.getAllDecrypted.mockResolvedValue([excludedCipherView]); - }); - - /** - * Spec: collect an authorization gesture confirming user consent for creating a new credential. - * Deviation: Consent is not asked and the user is simply informed of the situation. - **/ - it("should inform user", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - - try { - await authenticator.makeCredential(params); - // eslint-disable-next-line no-empty - } catch {} - - expect(userInterfaceSession.informExcludedCredential).toHaveBeenCalled(); - }); - - /** Spec: return an error code equivalent to "NotAllowedError" and terminate the operation. */ - it("should throw error", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - - const result = async () => await authenticator.makeCredential(params); - - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); - }); - - /** Devation: Organization ciphers are not checked against excluded credentials, even if the user has access to them. */ - it("should not inform user of duplication when the excluded credential belongs to an organization", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - excludedCipherView.organizationId = "someOrganizationId"; - - try { - await authenticator.makeCredential(params); - // eslint-disable-next-line no-empty - } catch {} - - expect(userInterfaceSession.informExcludedCredential).not.toHaveBeenCalled(); - }); - - it("should not inform user of duplication when input data does not pass checks", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - const invalidParams = await createInvalidParams(); - - for (const p of Object.values(invalidParams)) { - try { - await authenticator.makeCredential(p); - // eslint-disable-next-line no-empty - } catch {} - } - expect(userInterfaceSession.informExcludedCredential).not.toHaveBeenCalled(); - }); - - it.todo( - "should not throw error if the excluded credential has been marked as deleted in the vault" - ); - }); - describe("credential creation", () => { let existingCipher: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; beforeEach(async () => { existingCipher = createCipherView({ type: CipherType.Login }); - params = await createParams(); + params = await createParams({ requireResidentKey: false }); cipherService.get.mockImplementation(async (id) => id === existingCipher.id ? ({ decrypt: () => existingCipher } as any) : undefined ); @@ -321,6 +244,7 @@ describe("FidoAuthenticatorService", () => { userHandle: Fido2Utils.bufferToString(params.userEntity.id), counter: 0, userDisplayName: params.userEntity.displayName, + discoverable: false, }), }), }) @@ -498,7 +422,7 @@ describe("FidoAuthenticatorService", () => { * Spec: If requireUserVerification is true and the authenticator cannot perform user verification, return an error code equivalent to "ConstraintError" and terminate the operation. * Deviation: User verification is checked before checking for excluded credentials **/ - /** TODO: This test should only be activated if we disable support for user verification */ + /** NOTE: This test should only be activated if we disable support for user verification */ it.skip("should throw error if requireUserVerification is set to true", async () => { const params = await createParams({ requireUserVerification: true }); @@ -583,11 +507,11 @@ describe("FidoAuthenticatorService", () => { ciphers = [ await createCipherView( { type: CipherType.Login }, - { credentialId: credentialIds[0], rpId: RpId } + { credentialId: credentialIds[0], rpId: RpId, discoverable: false } ), await createCipherView( { type: CipherType.Login }, - { credentialId: credentialIds[1], rpId: RpId } + { credentialId: credentialIds[1], rpId: RpId, discoverable: true } ), ]; params = await createParams({ @@ -600,6 +524,36 @@ describe("FidoAuthenticatorService", () => { cipherService.getAllDecrypted.mockResolvedValue(ciphers); }); + it("should ask for all credentials in list when `params` contains allowedCredentials list", async () => { + userInterfaceSession.pickCredential.mockResolvedValue({ + cipherId: ciphers[0].id, + userVerified: false, + }); + + await authenticator.getAssertion(params); + + expect(userInterfaceSession.pickCredential).toHaveBeenCalledWith({ + cipherIds: ciphers.map((c) => c.id), + userVerification: false, + }); + }); + + it("should only ask for discoverable credentials matched by rpId when params does not contains allowedCredentials list", async () => { + params.allowCredentialDescriptorList = undefined; + const discoverableCiphers = ciphers.filter((c) => c.login.fido2Key.discoverable); + userInterfaceSession.pickCredential.mockResolvedValue({ + cipherId: discoverableCiphers[0].id, + userVerified: false, + }); + + await authenticator.getAssertion(params); + + expect(userInterfaceSession.pickCredential).toHaveBeenCalledWith({ + cipherIds: [discoverableCiphers[0].id], + userVerification: false, + }); + }); + for (const userVerification of [true, false]) { /** Spec: Prompt the user to select a public key credential source selectedCredential from credentialOptions. */ it(`should request confirmation from user when user verification is ${userVerification}`, async () => { @@ -787,7 +741,7 @@ function createCipherView( ): CipherView { const cipher = new CipherView(); cipher.id = data.id ?? Utils.newGuid(); - cipher.type = data.type ?? CipherType.Fido2Key; + cipher.type = CipherType.Login; cipher.localData = {}; const fido2KeyView = new Fido2KeyView(); @@ -797,6 +751,7 @@ function createCipherView( fido2KeyView.userHandle = fido2Key.userHandle ?? Fido2Utils.bufferToString(randomBytes(16)); fido2KeyView.keyAlgorithm = fido2Key.keyAlgorithm ?? "ECDSA"; fido2KeyView.keyCurve = fido2Key.keyCurve ?? "P-256"; + fido2KeyView.discoverable = fido2Key.discoverable ?? true; fido2KeyView.keyValue = fido2KeyView.keyValue ?? "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgTC-7XDZipXbaVBlnkjlBgO16ZmqBZWejK2iYo6lV0dehRANCAASOcM2WduNq1DriRYN7ZekvZz-bRhA-qNT4v0fbp5suUFJyWmgOQ0bybZcLXHaerK5Ep1JiSrQcewtQNgLtry7f"; diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index b945fcc972c..30f40158f27 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -188,8 +188,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr } let cipherOptions: CipherView[]; - - // eslint-disable-next-line no-empty if (params.allowCredentialDescriptorList?.length > 0) { cipherOptions = await this.findCredentialsById( params.allowCredentialDescriptorList, @@ -301,10 +299,9 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr (cipher) => !cipher.isDeleted && cipher.organizationId == undefined && - ((cipher.type === CipherType.Fido2Key && ids.includes(cipher.fido2Key.credentialId)) || - (cipher.type === CipherType.Login && - cipher.login.fido2Key != undefined && - ids.includes(cipher.login.fido2Key.credentialId))) + cipher.type === CipherType.Login && + cipher.login.fido2Key != undefined && + ids.includes(cipher.login.fido2Key.credentialId) ) .map((cipher) => cipher.id); } @@ -344,7 +341,8 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr !cipher.isDeleted && cipher.type === CipherType.Login && cipher.login.fido2Key != undefined && - cipher.login.fido2Key.rpId === rpId + cipher.login.fido2Key.rpId === rpId && + cipher.login.fido2Key.discoverable ); } } @@ -380,6 +378,7 @@ async function createKeyView( fido2Key.counter = 0; fido2Key.rpName = params.rpEntity.name; fido2Key.userDisplayName = params.userEntity.displayName; + fido2Key.discoverable = params.requireResidentKey; return fido2Key; }