From 3a293bbf1fec075685279617ccbca6d9f8c67594 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Wed, 13 Nov 2024 14:08:32 -0500 Subject: [PATCH] [PM-12399] Show card and identity ciphers in the autofill suggestions only if the active tab appears to have fields for that type of cipher (#11913) * show card and identity ciphers in the autofill suggestions only if the active tab appears to have fields for that type of cipher * handle cases where collectPageDetailsFromTab$ does not emit * update tests --- .../browser/src/background/main.background.ts | 6 ++- .../src/popup/services/services.module.ts | 2 + .../vault-popup-autofill.service.spec.ts | 18 +++++++ .../services/vault-popup-autofill.service.ts | 49 ++++++++++++++++++- .../vault-popup-items.service.spec.ts | 11 +++++ .../services/vault-popup-items.service.ts | 7 ++- 6 files changed, 89 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fb92ebe04a..fdfd5740ba 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -376,6 +376,7 @@ export default class MainBackground { autoSubmitLoginBackground: AutoSubmitLoginBackground; sdkService: SdkService; cipherAuthorizationService: CipherAuthorizationService; + inlineMenuFieldQualificationService: InlineMenuFieldQualificationService; onUpdatedRan: boolean; onReplacedRan: boolean; @@ -1249,6 +1250,8 @@ export default class MainBackground { this.collectionService, this.organizationService, ); + + this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); } async bootstrap() { @@ -1630,7 +1633,6 @@ export default class MainBackground { this.themeStateService, ); } else { - const inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); this.overlayBackground = new OverlayBackground( this.logService, this.cipherService, @@ -1643,7 +1645,7 @@ export default class MainBackground { this.platformUtilsService, this.vaultSettingsService, this.fido2ActiveRequestManager, - inlineMenuFieldQualificationService, + this.inlineMenuFieldQualificationService, this.themeStateService, this.totpService, () => this.generatePassword(), diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 6ef0c278dc..919cf8c0b1 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -113,6 +113,7 @@ import { ExtensionAnonLayoutWrapperDataService } from "../../auth/popup/extensio import { ExtensionLoginComponentService } from "../../auth/popup/login/extension-login-component.service"; import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service"; import AutofillService from "../../autofill/services/autofill.service"; +import { InlineMenuFieldQualificationService } from "../../autofill/services/inline-menu-field-qualification.service"; import { ForegroundBrowserBiometricsService } from "../../key-management/biometrics/foreground-browser-biometrics"; import { BrowserKeyService } from "../../key-management/browser-key.service"; import { BrowserApi } from "../../platform/browser/browser-api"; @@ -167,6 +168,7 @@ const safeProviders: SafeProvider[] = [ safeProvider(DebounceNavigationService), safeProvider(DialogService), safeProvider(PopupCloseWarningService), + safeProvider(InlineMenuFieldQualificationService), safeProvider({ provide: DEFAULT_VAULT_TIMEOUT, useValue: VaultTimeoutStringType.OnRestart, diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts index effadad07f..25a7d7594a 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts @@ -5,6 +5,7 @@ import { BehaviorSubject, of } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -19,6 +20,7 @@ import { LoginView } from "@bitwarden/common/vault/models/view/login.view"; import { ToastService } from "@bitwarden/components"; import { PasswordRepromptService } from "@bitwarden/vault"; +import { InlineMenuFieldQualificationService } from "../../../../../browser/src/autofill/services/inline-menu-field-qualification.service"; import { AutoFillOptions, AutofillService, @@ -46,6 +48,8 @@ describe("VaultPopupAutofillService", () => { const mockPasswordRepromptService = mock(); const mockCipherService = mock(); const mockMessagingService = mock(); + const mockInlineMenuFieldQualificationService = mock(); + const mockLogService = mock(); const mockUserId = Utils.newGuid() as UserId; const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); @@ -53,6 +57,12 @@ describe("VaultPopupAutofillService", () => { beforeEach(() => { jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false); jest.spyOn(BrowserApi, "getTabFromCurrentWindow").mockResolvedValue(mockCurrentTab); + jest + .spyOn(mockInlineMenuFieldQualificationService, "isFieldForCreditCardForm") + .mockReturnValue(true); + jest + .spyOn(mockInlineMenuFieldQualificationService, "isFieldForIdentityForm") + .mockReturnValue(true); mockAutofillService.collectPageDetailsFromTab$.mockReturnValue(new BehaviorSubject([])); @@ -70,6 +80,14 @@ describe("VaultPopupAutofillService", () => { provide: AccountService, useValue: accountService, }, + { + provide: InlineMenuFieldQualificationService, + useValue: mockInlineMenuFieldQualificationService, + }, + { + provide: LogService, + useValue: mockLogService, + }, ], }); diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts index a2e032a54f..0f76c2f2cd 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts @@ -10,10 +10,12 @@ import { startWith, Subject, switchMap, + timeout, } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -23,6 +25,7 @@ import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view import { ToastService } from "@bitwarden/components"; import { PasswordRepromptService } from "@bitwarden/vault"; +import { InlineMenuFieldQualificationService } from "../../../../../browser/src/autofill/services/inline-menu-field-qualification.service"; import { AutofillService, PageDetail, @@ -72,11 +75,53 @@ export class VaultPopupAutofillService { if (!tab) { return of([]); } - return this.autofillService.collectPageDetailsFromTab$(tab); + return this.autofillService + .collectPageDetailsFromTab$(tab) + .pipe(timeout({ first: 1500, with: () => of([]) })); }), shareReplay({ refCount: false, bufferSize: 1 }), ); + nonLoginCipherTypesOnPage$: Observable<{ + [CipherType.Card]: boolean; + [CipherType.Identity]: boolean; + }> = this._currentPageDetails$.pipe( + map((pageDetails) => { + let pageHasCardFields = false; + let pageHasIdentityFields = false; + + try { + if (!pageDetails) { + throw Error("No page details were provided"); + } + + for (const details of pageDetails) { + for (const field of details.details.fields) { + if (!pageHasCardFields) { + pageHasCardFields = this.inlineMenuFieldQualificationService.isFieldForCreditCardForm( + field, + details.details, + ); + } + + if (!pageHasIdentityFields) { + pageHasIdentityFields = + this.inlineMenuFieldQualificationService.isFieldForIdentityForm( + field, + details.details, + ); + } + } + } + } catch (error) { + // no-op on failure; do not show extra cipher types + this.logService.warning(error.message); + } + + return { [CipherType.Card]: pageHasCardFields, [CipherType.Identity]: pageHasIdentityFields }; + }), + ); + constructor( private autofillService: AutofillService, private i18nService: I18nService, @@ -87,6 +132,8 @@ export class VaultPopupAutofillService { private messagingService: MessagingService, private route: ActivatedRoute, private accountService: AccountService, + private logService: LogService, + private inlineMenuFieldQualificationService: InlineMenuFieldQualificationService, ) { this._currentPageDetails$.subscribe(); } diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 610d48fdc6..1900d35d9d 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -15,6 +15,7 @@ import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { InlineMenuFieldQualificationService } from "../../../../../browser/src/autofill/services/inline-menu-field-qualification.service"; import { BrowserApi } from "../../../platform/browser/browser-api"; import { VaultPopupAutofillService } from "./vault-popup-autofill.service"; @@ -39,6 +40,7 @@ describe("VaultPopupItemsService", () => { const collectionService = mock(); const vaultAutofillServiceMock = mock(); const syncServiceMock = mock(); + const inlineMenuFieldQualificationServiceMock = mock(); beforeEach(() => { allCiphers = cipherFactory(10); @@ -78,6 +80,11 @@ describe("VaultPopupItemsService", () => { url: "https://example.com", } as chrome.tabs.Tab); + vaultAutofillServiceMock.nonLoginCipherTypesOnPage$ = new BehaviorSubject({ + [CipherType.Card]: true, + [CipherType.Identity]: true, + }); + mockOrg = { id: "org1", name: "Organization 1", @@ -105,6 +112,10 @@ describe("VaultPopupItemsService", () => { { provide: CollectionService, useValue: collectionService }, { provide: VaultPopupAutofillService, useValue: vaultAutofillServiceMock }, { provide: SyncService, useValue: syncServiceMock }, + { + provide: InlineMenuFieldQualificationService, + useValue: inlineMenuFieldQualificationServiceMock, + }, ], }); diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 20ac3b3de9..6c3652425c 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -62,8 +62,13 @@ export class VaultPopupItemsService { private _otherAutoFillTypes$: Observable = combineLatest([ this.vaultSettingsService.showCardsCurrentTab$, this.vaultSettingsService.showIdentitiesCurrentTab$, + this.vaultPopupAutofillService.nonLoginCipherTypesOnPage$, ]).pipe( - map(([showCards, showIdentities]) => { + map(([showCardsSettingEnabled, showIdentitiesSettingEnabled, nonLoginCipherTypesOnPage]) => { + const showCards = showCardsSettingEnabled && nonLoginCipherTypesOnPage[CipherType.Card]; + const showIdentities = + showIdentitiesSettingEnabled && nonLoginCipherTypesOnPage[CipherType.Identity]; + return [ ...(showCards ? [CipherType.Card] : []), ...(showIdentities ? [CipherType.Identity] : []),