[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
This commit is contained in:
Cesar Gonzalez 2024-08-22 12:32:31 -05:00 committed by GitHub
parent 898338ff9d
commit c782c92f6c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 46 additions and 11 deletions

View File

@ -40,6 +40,7 @@ export type FocusedFieldData = {
frameId?: number; frameId?: number;
accountCreationFieldType?: string; accountCreationFieldType?: string;
showInlineMenuAccountCreation?: boolean; showInlineMenuAccountCreation?: boolean;
showPasskeys?: boolean;
}; };
export type InlineMenuElementPosition = { export type InlineMenuElementPosition = {
@ -211,6 +212,7 @@ export type OverlayBackgroundExtensionMessageHandlers = {
}: BackgroundOnMessageHandlerParams) => void; }: BackgroundOnMessageHandlerParams) => void;
collectPageDetailsResponse: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; collectPageDetailsResponse: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
unlockCompleted: ({ message }: BackgroundMessageParam) => void; unlockCompleted: ({ message }: BackgroundMessageParam) => void;
doFullSync: () => void;
addedCipher: () => void; addedCipher: () => void;
addEditCipherSubmitted: () => void; addEditCipherSubmitted: () => void;
editedCipher: () => void; editedCipher: () => void;

View File

@ -717,7 +717,7 @@ describe("OverlayBackground", () => {
localData: { lastUsedDate: 222 }, localData: { lastUsedDate: 222 },
name: "name-1", name: "name-1",
type: CipherType.Login, type: CipherType.Login,
login: { username: "username-1", uri: url }, login: { username: "username-1", password: "password", uri: url },
}); });
const cardCipher = mock<CipherView>({ const cardCipher = mock<CipherView>({
id: "id-2", id: "id-2",
@ -752,6 +752,7 @@ describe("OverlayBackground", () => {
type: CipherType.Login, type: CipherType.Login,
login: { login: {
username: "username-5", username: "username-5",
password: "password",
uri: url, uri: url,
fido2Credentials: [ fido2Credentials: [
mock<Fido2CredentialView>({ mock<Fido2CredentialView>({
@ -1116,6 +1117,7 @@ describe("OverlayBackground", () => {
overlayBackground["focusedFieldData"] = createFocusedFieldDataMock({ overlayBackground["focusedFieldData"] = createFocusedFieldDataMock({
tabId: tab.id, tabId: tab.id,
filledByCipherType: CipherType.Login, filledByCipherType: CipherType.Login,
showPasskeys: true,
}); });
cipherService.getAllDecryptedForUrl.mockResolvedValue([loginCipher1, passkeyCipher]); cipherService.getAllDecryptedForUrl.mockResolvedValue([loginCipher1, passkeyCipher]);
cipherService.sortCiphersByLastUsedThenName.mockReturnValue(-1); cipherService.sortCiphersByLastUsedThenName.mockReturnValue(-1);
@ -2517,6 +2519,7 @@ describe("OverlayBackground", () => {
describe("extension messages that trigger an update of the inline menu ciphers", () => { describe("extension messages that trigger an update of the inline menu ciphers", () => {
const extensionMessages = [ const extensionMessages = [
"doFullSync",
"addedCipher", "addedCipher",
"addEditCipherSubmitted", "addEditCipherSubmitted",
"editedCipher", "editedCipher",

View File

@ -139,6 +139,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId), this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId),
collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender), collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender),
unlockCompleted: ({ message }) => this.unlockCompleted(message), unlockCompleted: ({ message }) => this.unlockCompleted(message),
doFullSync: () => this.updateOverlayCiphers(true),
addedCipher: () => this.updateOverlayCiphers(), addedCipher: () => this.updateOverlayCiphers(),
addEditCipherSubmitted: () => this.updateOverlayCiphers(), addEditCipherSubmitted: () => this.updateOverlayCiphers(),
editedCipher: () => this.updateOverlayCiphers(), editedCipher: () => this.updateOverlayCiphers(),
@ -455,7 +456,13 @@ export class OverlayBackground implements OverlayBackgroundInterface {
continue; continue;
} }
if (this.showCipherAsPasskey(cipher, domainExclusionsSet)) { if (!this.showCipherAsPasskey(cipher, domainExclusionsSet)) {
inlineMenuCipherData.push(
this.buildCipherData({ inlineMenuCipherId, cipher, showFavicons }),
);
continue;
}
passkeyCipherData.push( passkeyCipherData.push(
this.buildCipherData({ this.buildCipherData({
inlineMenuCipherId, inlineMenuCipherId,
@ -464,9 +471,12 @@ export class OverlayBackground implements OverlayBackgroundInterface {
hasPasskey: true, hasPasskey: true,
}), }),
); );
}
inlineMenuCipherData.push(this.buildCipherData({ inlineMenuCipherId, cipher, showFavicons })); if (cipher.login?.password && cipher.login.username) {
inlineMenuCipherData.push(
this.buildCipherData({ inlineMenuCipherId, cipher, showFavicons }),
);
}
} }
if (passkeyCipherData.length) { if (passkeyCipherData.length) {
@ -485,7 +495,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
* @param domainExclusions - The domain exclusions to check against * @param domainExclusions - The domain exclusions to check against
*/ */
private showCipherAsPasskey(cipher: CipherView, domainExclusions: Set<string> | null): boolean { private showCipherAsPasskey(cipher: CipherView, domainExclusions: Set<string> | null): boolean {
if (cipher.type !== CipherType.Login) { if (cipher.type !== CipherType.Login || !this.focusedFieldData?.showPasskeys) {
return false; return false;
} }

View File

@ -241,7 +241,16 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
async pickCredential({ async pickCredential({
cipherIds, cipherIds,
userVerification, userVerification,
assumeUserPresence,
}: PickCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> { }: 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 = { const data: BrowserFido2Message = {
type: BrowserFido2MessageTypes.PickCredentialRequest, type: BrowserFido2MessageTypes.PickCredentialRequest,
cipherIds, cipherIds,

View File

@ -115,5 +115,7 @@ export default class AutofillField {
showInlineMenuAccountCreation?: boolean; showInlineMenuAccountCreation?: boolean;
showPasskeys?: boolean;
fieldQualifier?: AutofillFieldQualifierType; fieldQualifier?: AutofillFieldQualifierType;
} }

View File

@ -792,6 +792,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
focusedFieldRects: { width, height, top, left }, focusedFieldRects: { width, height, top, left },
filledByCipherType: autofillFieldData?.filledByCipherType, filledByCipherType: autofillFieldData?.filledByCipherType,
showInlineMenuAccountCreation: autofillFieldData?.showInlineMenuAccountCreation, showInlineMenuAccountCreation: autofillFieldData?.showInlineMenuAccountCreation,
showPasskeys: !!autofillFieldData?.showPasskeys,
accountCreationFieldType, accountCreationFieldType,
}; };
@ -874,6 +875,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
this.inlineMenuFieldQualificationService.isFieldForLoginForm(autofillFieldData, pageDetails) this.inlineMenuFieldQualificationService.isFieldForLoginForm(autofillFieldData, pageDetails)
) { ) {
autofillFieldData.filledByCipherType = CipherType.Login; autofillFieldData.filledByCipherType = CipherType.Login;
autofillFieldData.showPasskeys = autofillFieldData.autoCompleteType.includes("webauthn");
return false; return false;
} }

View File

@ -40,6 +40,11 @@ export interface PickCredentialParams {
* Whether or not the user must be verified before completing the operation. * Whether or not the user must be verified before completing the operation.
*/ */
userVerification: boolean; userVerification: boolean;
/**
* Bypass the UI and assume that the user has already interacted with the authenticator.
*/
assumeUserPresence?: boolean;
} }
/** /**

View File

@ -243,10 +243,12 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
} }
let response = { cipherId: cipherOptions[0].id, userVerified: false }; let response = { cipherId: cipherOptions[0].id, userVerified: false };
if (this.requiresUserVerificationPrompt(params, cipherOptions)) { if (this.requiresUserVerificationPrompt(params, cipherOptions)) {
response = await userInterfaceSession.pickCredential({ response = await userInterfaceSession.pickCredential({
cipherIds: cipherOptions.map((cipher) => cipher.id), cipherIds: cipherOptions.map((cipher) => cipher.id),
userVerification: params.requireUserVerification, userVerification: params.requireUserVerification,
assumeUserPresence: params.assumeUserPresence,
}); });
} }