From f571c490edf7d14b0045ad3fe3cd512298c86267 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Thu, 7 Nov 2024 17:54:16 -0500 Subject: [PATCH 1/3] show card and identity ciphers in the autofill suggestions only if the active tab appears to have fields for that type of cipher --- .../browser/src/background/main.background.ts | 6 ++- .../src/popup/services/services.module.ts | 5 +++ .../services/vault-popup-autofill.service.ts | 37 +++++++++++++++++++ .../services/vault-popup-items.service.ts | 7 +++- 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fb92ebe04ae..ae907b2ea71 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; @@ -704,6 +705,8 @@ export default class MainBackground { this.stateProvider, ); + this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); + this.autofillSettingsService = new AutofillSettingsService( this.stateProvider, this.policyService, @@ -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 6ef0c278dc3..af59aba7fb9 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"; @@ -349,6 +350,10 @@ const safeProviders: SafeProvider[] = [ MessageListener, ], }), + safeProvider({ + provide: InlineMenuFieldQualificationService, + deps: [], + }), safeProvider({ provide: ScriptInjectorService, useClass: BrowserScriptInjectorService, 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 a2e032a54f1..2b5f35a8915 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 @@ -14,6 +14,7 @@ import { 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 +24,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, @@ -77,6 +79,39 @@ export class VaultPopupAutofillService { 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 { + for (const details of pageDetails) { + for (const field of details.details.fields) { + if (!pageHasCardFields && !pageHasIdentityFields) { + pageHasCardFields = this.inlineMenuFieldQualificationService.isFieldForCreditCardForm( + field, + details.details, + ); + 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 +122,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.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 20ac3b3de96..6c3652425c9 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] : []), From 8ea5b4821e8ec4a48eec06be4664f46b0aac3687 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Fri, 8 Nov 2024 13:10:56 -0500 Subject: [PATCH 2/3] handle cases where collectPageDetailsFromTab$ does not emit --- apps/browser/src/background/main.background.ts | 4 ++-- apps/browser/src/popup/services/services.module.ts | 5 +---- .../popup/services/vault-popup-autofill.service.ts | 14 ++++++++++++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index ae907b2ea71..fdfd5740ba0 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -705,8 +705,6 @@ export default class MainBackground { this.stateProvider, ); - this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); - this.autofillSettingsService = new AutofillSettingsService( this.stateProvider, this.policyService, @@ -1252,6 +1250,8 @@ export default class MainBackground { this.collectionService, this.organizationService, ); + + this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); } async bootstrap() { diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index af59aba7fb9..919cf8c0b19 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -168,6 +168,7 @@ const safeProviders: SafeProvider[] = [ safeProvider(DebounceNavigationService), safeProvider(DialogService), safeProvider(PopupCloseWarningService), + safeProvider(InlineMenuFieldQualificationService), safeProvider({ provide: DEFAULT_VAULT_TIMEOUT, useValue: VaultTimeoutStringType.OnRestart, @@ -350,10 +351,6 @@ const safeProviders: SafeProvider[] = [ MessageListener, ], }), - safeProvider({ - provide: InlineMenuFieldQualificationService, - deps: [], - }), safeProvider({ provide: ScriptInjectorService, useClass: BrowserScriptInjectorService, 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 2b5f35a8915..0f76c2f2cdb 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,6 +10,7 @@ import { startWith, Subject, switchMap, + timeout, } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -74,7 +75,9 @@ 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 }), ); @@ -88,13 +91,20 @@ export class VaultPopupAutofillService { 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 && !pageHasIdentityFields) { + if (!pageHasCardFields) { pageHasCardFields = this.inlineMenuFieldQualificationService.isFieldForCreditCardForm( field, details.details, ); + } + + if (!pageHasIdentityFields) { pageHasIdentityFields = this.inlineMenuFieldQualificationService.isFieldForIdentityForm( field, From 6dc73b8e01116269932a2e92c3dd572c64f5f0c0 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Wed, 13 Nov 2024 11:27:40 -0500 Subject: [PATCH 3/3] update tests --- .../vault-popup-autofill.service.spec.ts | 18 ++++++++++++++++++ .../services/vault-popup-items.service.spec.ts | 11 +++++++++++ 2 files changed, 29 insertions(+) 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 effadad07fb..25a7d7594ae 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-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 610d48fdc6f..1900d35d9d9 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, + }, ], });