From c782c92f6ce72497f0d19862418333e8d5d57bfb Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Thu, 22 Aug 2024 12:32:31 -0500 Subject: [PATCH] [PM-11170] Inline menu should not show incomplete login item along passkey (#10635) * [PM-11170] Inline menu should not show incomplete login items along with passkeys * [PM-11170] Inline menu should not show incomplete login items along with passkeys * [PM-11170] Incorporating a fix for ciphers not being updated within the inline menu on user actions * [PM-11170] Ensuring that conditional mediated auth does not trigger the authenticator * [PM-11170] Adding a jest test to verify conditional mediated UI calls do not trigger the authenticator * [PM-11170] Adding a jest test to verify conditional mediated UI calls do not trigger the authenticator * [PM-11170] Reworking implementation to have the BrowserFido2UserInterfaceService trigger the expected behavior for the conditional mediated auth * [PM-11170] Reworking implementation to have the BrowserFido2UserInterfaceService trigger the expected behavior for the conditional mediated auth * [PM-11170] Following up on feedback provided during code review, reworking inline menu bypass of the fido2 authenticator to function based on assumeUserPresence param when triggering from the inline menu * [PM-11170] Following up on feedback provided during code review, reworking inline menu bypass of the fido2 authenticator to function based on assumeUserPresence param when triggering from the inline menu * [PM-11170] Following up on feedback provided during code review, reworking inline menu bypass of the fido2 authenticator to function based on assumeUserPresence param when triggering from the inline menu --- .../abstractions/overlay.background.ts | 2 ++ .../background/overlay.background.spec.ts | 5 +++- .../autofill/background/overlay.background.ts | 30 ++++++++++++------- .../browser-fido2-user-interface.service.ts | 9 ++++++ .../src/autofill/models/autofill-field.ts | 2 ++ .../autofill-overlay-content.service.ts | 2 ++ ...ido2-user-interface.service.abstraction.ts | 5 ++++ .../fido2/fido2-authenticator.service.ts | 2 ++ 8 files changed, 46 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/overlay.background.ts b/apps/browser/src/autofill/background/abstractions/overlay.background.ts index 950f3b8e27..5d2d2a3898 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts @@ -40,6 +40,7 @@ export type FocusedFieldData = { frameId?: number; accountCreationFieldType?: string; showInlineMenuAccountCreation?: boolean; + showPasskeys?: boolean; }; export type InlineMenuElementPosition = { @@ -211,6 +212,7 @@ export type OverlayBackgroundExtensionMessageHandlers = { }: BackgroundOnMessageHandlerParams) => void; collectPageDetailsResponse: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; unlockCompleted: ({ message }: BackgroundMessageParam) => void; + doFullSync: () => void; addedCipher: () => void; addEditCipherSubmitted: () => void; editedCipher: () => void; diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index b2ad7128b6..5ac94582ce 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -717,7 +717,7 @@ describe("OverlayBackground", () => { localData: { lastUsedDate: 222 }, name: "name-1", type: CipherType.Login, - login: { username: "username-1", uri: url }, + login: { username: "username-1", password: "password", uri: url }, }); const cardCipher = mock({ id: "id-2", @@ -752,6 +752,7 @@ describe("OverlayBackground", () => { type: CipherType.Login, login: { username: "username-5", + password: "password", uri: url, fido2Credentials: [ mock({ @@ -1116,6 +1117,7 @@ describe("OverlayBackground", () => { overlayBackground["focusedFieldData"] = createFocusedFieldDataMock({ tabId: tab.id, filledByCipherType: CipherType.Login, + showPasskeys: true, }); cipherService.getAllDecryptedForUrl.mockResolvedValue([loginCipher1, passkeyCipher]); cipherService.sortCiphersByLastUsedThenName.mockReturnValue(-1); @@ -2517,6 +2519,7 @@ describe("OverlayBackground", () => { describe("extension messages that trigger an update of the inline menu ciphers", () => { const extensionMessages = [ + "doFullSync", "addedCipher", "addEditCipherSubmitted", "editedCipher", diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 6e7db21a6d..a209523dc7 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -139,6 +139,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId), collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender), unlockCompleted: ({ message }) => this.unlockCompleted(message), + doFullSync: () => this.updateOverlayCiphers(true), addedCipher: () => this.updateOverlayCiphers(), addEditCipherSubmitted: () => this.updateOverlayCiphers(), editedCipher: () => this.updateOverlayCiphers(), @@ -455,18 +456,27 @@ export class OverlayBackground implements OverlayBackgroundInterface { continue; } - if (this.showCipherAsPasskey(cipher, domainExclusionsSet)) { - passkeyCipherData.push( - this.buildCipherData({ - inlineMenuCipherId, - cipher, - showFavicons, - hasPasskey: true, - }), + if (!this.showCipherAsPasskey(cipher, domainExclusionsSet)) { + inlineMenuCipherData.push( + this.buildCipherData({ inlineMenuCipherId, cipher, showFavicons }), ); + continue; } - inlineMenuCipherData.push(this.buildCipherData({ inlineMenuCipherId, cipher, showFavicons })); + passkeyCipherData.push( + this.buildCipherData({ + inlineMenuCipherId, + cipher, + showFavicons, + hasPasskey: true, + }), + ); + + if (cipher.login?.password && cipher.login.username) { + inlineMenuCipherData.push( + this.buildCipherData({ inlineMenuCipherId, cipher, showFavicons }), + ); + } } if (passkeyCipherData.length) { @@ -485,7 +495,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { * @param domainExclusions - The domain exclusions to check against */ private showCipherAsPasskey(cipher: CipherView, domainExclusions: Set | null): boolean { - if (cipher.type !== CipherType.Login) { + if (cipher.type !== CipherType.Login || !this.focusedFieldData?.showPasskeys) { return false; } diff --git a/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts b/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts index f373494d52..1e5e880c67 100644 --- a/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts @@ -241,7 +241,16 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi async pickCredential({ cipherIds, userVerification, + assumeUserPresence, }: PickCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> { + // NOTE: For now, we are defaulting to a userVerified status of `true` when the request + // is for a conditionally mediated authentication. This will allow for mediated conditional + // authentication to function without requiring user interaction. This is a product + // decision, rather than a decision based on the expected technical specifications. + if (assumeUserPresence && cipherIds.length === 1) { + return { cipherId: cipherIds[0], userVerified: userVerification }; + } + const data: BrowserFido2Message = { type: BrowserFido2MessageTypes.PickCredentialRequest, cipherIds, diff --git a/apps/browser/src/autofill/models/autofill-field.ts b/apps/browser/src/autofill/models/autofill-field.ts index 5a95b92899..0701ef5f65 100644 --- a/apps/browser/src/autofill/models/autofill-field.ts +++ b/apps/browser/src/autofill/models/autofill-field.ts @@ -115,5 +115,7 @@ export default class AutofillField { showInlineMenuAccountCreation?: boolean; + showPasskeys?: boolean; + fieldQualifier?: AutofillFieldQualifierType; } diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts index 5b641e46c1..d0d1a79e29 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -792,6 +792,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ focusedFieldRects: { width, height, top, left }, filledByCipherType: autofillFieldData?.filledByCipherType, showInlineMenuAccountCreation: autofillFieldData?.showInlineMenuAccountCreation, + showPasskeys: !!autofillFieldData?.showPasskeys, accountCreationFieldType, }; @@ -874,6 +875,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ this.inlineMenuFieldQualificationService.isFieldForLoginForm(autofillFieldData, pageDetails) ) { autofillFieldData.filledByCipherType = CipherType.Login; + autofillFieldData.showPasskeys = autofillFieldData.autoCompleteType.includes("webauthn"); return false; } diff --git a/libs/common/src/platform/abstractions/fido2/fido2-user-interface.service.abstraction.ts b/libs/common/src/platform/abstractions/fido2/fido2-user-interface.service.abstraction.ts index 9882febdd3..55232162d4 100644 --- a/libs/common/src/platform/abstractions/fido2/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/platform/abstractions/fido2/fido2-user-interface.service.abstraction.ts @@ -40,6 +40,11 @@ export interface PickCredentialParams { * Whether or not the user must be verified before completing the operation. */ userVerification: boolean; + + /** + * Bypass the UI and assume that the user has already interacted with the authenticator. + */ + assumeUserPresence?: boolean; } /** 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 08998b965f..ef09a3d160 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts @@ -243,10 +243,12 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr } let response = { cipherId: cipherOptions[0].id, userVerified: false }; + if (this.requiresUserVerificationPrompt(params, cipherOptions)) { response = await userInterfaceSession.pickCredential({ cipherIds: cipherOptions.map((cipher) => cipher.id), userVerification: params.requireUserVerification, + assumeUserPresence: params.assumeUserPresence, }); }