1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-14 10:26:19 +01:00

[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
This commit is contained in:
Jonathan Prusik 2024-11-13 14:08:32 -05:00 committed by GitHub
parent 204eb3105b
commit 3a293bbf1f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 89 additions and 4 deletions

View File

@ -376,6 +376,7 @@ export default class MainBackground {
autoSubmitLoginBackground: AutoSubmitLoginBackground; autoSubmitLoginBackground: AutoSubmitLoginBackground;
sdkService: SdkService; sdkService: SdkService;
cipherAuthorizationService: CipherAuthorizationService; cipherAuthorizationService: CipherAuthorizationService;
inlineMenuFieldQualificationService: InlineMenuFieldQualificationService;
onUpdatedRan: boolean; onUpdatedRan: boolean;
onReplacedRan: boolean; onReplacedRan: boolean;
@ -1249,6 +1250,8 @@ export default class MainBackground {
this.collectionService, this.collectionService,
this.organizationService, this.organizationService,
); );
this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService();
} }
async bootstrap() { async bootstrap() {
@ -1630,7 +1633,6 @@ export default class MainBackground {
this.themeStateService, this.themeStateService,
); );
} else { } else {
const inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService();
this.overlayBackground = new OverlayBackground( this.overlayBackground = new OverlayBackground(
this.logService, this.logService,
this.cipherService, this.cipherService,
@ -1643,7 +1645,7 @@ export default class MainBackground {
this.platformUtilsService, this.platformUtilsService,
this.vaultSettingsService, this.vaultSettingsService,
this.fido2ActiveRequestManager, this.fido2ActiveRequestManager,
inlineMenuFieldQualificationService, this.inlineMenuFieldQualificationService,
this.themeStateService, this.themeStateService,
this.totpService, this.totpService,
() => this.generatePassword(), () => this.generatePassword(),

View File

@ -113,6 +113,7 @@ import { ExtensionAnonLayoutWrapperDataService } from "../../auth/popup/extensio
import { ExtensionLoginComponentService } from "../../auth/popup/login/extension-login-component.service"; import { ExtensionLoginComponentService } from "../../auth/popup/login/extension-login-component.service";
import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service"; import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service";
import AutofillService from "../../autofill/services/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 { ForegroundBrowserBiometricsService } from "../../key-management/biometrics/foreground-browser-biometrics";
import { BrowserKeyService } from "../../key-management/browser-key.service"; import { BrowserKeyService } from "../../key-management/browser-key.service";
import { BrowserApi } from "../../platform/browser/browser-api"; import { BrowserApi } from "../../platform/browser/browser-api";
@ -167,6 +168,7 @@ const safeProviders: SafeProvider[] = [
safeProvider(DebounceNavigationService), safeProvider(DebounceNavigationService),
safeProvider(DialogService), safeProvider(DialogService),
safeProvider(PopupCloseWarningService), safeProvider(PopupCloseWarningService),
safeProvider(InlineMenuFieldQualificationService),
safeProvider({ safeProvider({
provide: DEFAULT_VAULT_TIMEOUT, provide: DEFAULT_VAULT_TIMEOUT,
useValue: VaultTimeoutStringType.OnRestart, useValue: VaultTimeoutStringType.OnRestart,

View File

@ -5,6 +5,7 @@ import { BehaviorSubject, of } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.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 { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { Utils } from "@bitwarden/common/platform/misc/utils"; 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 { ToastService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault"; import { PasswordRepromptService } from "@bitwarden/vault";
import { InlineMenuFieldQualificationService } from "../../../../../browser/src/autofill/services/inline-menu-field-qualification.service";
import { import {
AutoFillOptions, AutoFillOptions,
AutofillService, AutofillService,
@ -46,6 +48,8 @@ describe("VaultPopupAutofillService", () => {
const mockPasswordRepromptService = mock<PasswordRepromptService>(); const mockPasswordRepromptService = mock<PasswordRepromptService>();
const mockCipherService = mock<CipherService>(); const mockCipherService = mock<CipherService>();
const mockMessagingService = mock<MessagingService>(); const mockMessagingService = mock<MessagingService>();
const mockInlineMenuFieldQualificationService = mock<InlineMenuFieldQualificationService>();
const mockLogService = mock<LogService>();
const mockUserId = Utils.newGuid() as UserId; const mockUserId = Utils.newGuid() as UserId;
const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); const accountService: FakeAccountService = mockAccountServiceWith(mockUserId);
@ -53,6 +57,12 @@ describe("VaultPopupAutofillService", () => {
beforeEach(() => { beforeEach(() => {
jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false); jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false);
jest.spyOn(BrowserApi, "getTabFromCurrentWindow").mockResolvedValue(mockCurrentTab); jest.spyOn(BrowserApi, "getTabFromCurrentWindow").mockResolvedValue(mockCurrentTab);
jest
.spyOn(mockInlineMenuFieldQualificationService, "isFieldForCreditCardForm")
.mockReturnValue(true);
jest
.spyOn(mockInlineMenuFieldQualificationService, "isFieldForIdentityForm")
.mockReturnValue(true);
mockAutofillService.collectPageDetailsFromTab$.mockReturnValue(new BehaviorSubject([])); mockAutofillService.collectPageDetailsFromTab$.mockReturnValue(new BehaviorSubject([]));
@ -70,6 +80,14 @@ describe("VaultPopupAutofillService", () => {
provide: AccountService, provide: AccountService,
useValue: accountService, useValue: accountService,
}, },
{
provide: InlineMenuFieldQualificationService,
useValue: mockInlineMenuFieldQualificationService,
},
{
provide: LogService,
useValue: mockLogService,
},
], ],
}); });

View File

@ -10,10 +10,12 @@ import {
startWith, startWith,
Subject, Subject,
switchMap, switchMap,
timeout,
} from "rxjs"; } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.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 { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.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 { ToastService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault"; import { PasswordRepromptService } from "@bitwarden/vault";
import { InlineMenuFieldQualificationService } from "../../../../../browser/src/autofill/services/inline-menu-field-qualification.service";
import { import {
AutofillService, AutofillService,
PageDetail, PageDetail,
@ -72,11 +75,53 @@ export class VaultPopupAutofillService {
if (!tab) { if (!tab) {
return of([]); return of([]);
} }
return this.autofillService.collectPageDetailsFromTab$(tab); return this.autofillService
.collectPageDetailsFromTab$(tab)
.pipe(timeout({ first: 1500, with: () => of([]) }));
}), }),
shareReplay({ refCount: false, bufferSize: 1 }), 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( constructor(
private autofillService: AutofillService, private autofillService: AutofillService,
private i18nService: I18nService, private i18nService: I18nService,
@ -87,6 +132,8 @@ export class VaultPopupAutofillService {
private messagingService: MessagingService, private messagingService: MessagingService,
private route: ActivatedRoute, private route: ActivatedRoute,
private accountService: AccountService, private accountService: AccountService,
private logService: LogService,
private inlineMenuFieldQualificationService: InlineMenuFieldQualificationService,
) { ) {
this._currentPageDetails$.subscribe(); this._currentPageDetails$.subscribe();
} }

View File

@ -15,6 +15,7 @@ import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault
import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; 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 { BrowserApi } from "../../../platform/browser/browser-api";
import { VaultPopupAutofillService } from "./vault-popup-autofill.service"; import { VaultPopupAutofillService } from "./vault-popup-autofill.service";
@ -39,6 +40,7 @@ describe("VaultPopupItemsService", () => {
const collectionService = mock<CollectionService>(); const collectionService = mock<CollectionService>();
const vaultAutofillServiceMock = mock<VaultPopupAutofillService>(); const vaultAutofillServiceMock = mock<VaultPopupAutofillService>();
const syncServiceMock = mock<SyncService>(); const syncServiceMock = mock<SyncService>();
const inlineMenuFieldQualificationServiceMock = mock<InlineMenuFieldQualificationService>();
beforeEach(() => { beforeEach(() => {
allCiphers = cipherFactory(10); allCiphers = cipherFactory(10);
@ -78,6 +80,11 @@ describe("VaultPopupItemsService", () => {
url: "https://example.com", url: "https://example.com",
} as chrome.tabs.Tab); } as chrome.tabs.Tab);
vaultAutofillServiceMock.nonLoginCipherTypesOnPage$ = new BehaviorSubject({
[CipherType.Card]: true,
[CipherType.Identity]: true,
});
mockOrg = { mockOrg = {
id: "org1", id: "org1",
name: "Organization 1", name: "Organization 1",
@ -105,6 +112,10 @@ describe("VaultPopupItemsService", () => {
{ provide: CollectionService, useValue: collectionService }, { provide: CollectionService, useValue: collectionService },
{ provide: VaultPopupAutofillService, useValue: vaultAutofillServiceMock }, { provide: VaultPopupAutofillService, useValue: vaultAutofillServiceMock },
{ provide: SyncService, useValue: syncServiceMock }, { provide: SyncService, useValue: syncServiceMock },
{
provide: InlineMenuFieldQualificationService,
useValue: inlineMenuFieldQualificationServiceMock,
},
], ],
}); });

View File

@ -62,8 +62,13 @@ export class VaultPopupItemsService {
private _otherAutoFillTypes$: Observable<CipherType[]> = combineLatest([ private _otherAutoFillTypes$: Observable<CipherType[]> = combineLatest([
this.vaultSettingsService.showCardsCurrentTab$, this.vaultSettingsService.showCardsCurrentTab$,
this.vaultSettingsService.showIdentitiesCurrentTab$, this.vaultSettingsService.showIdentitiesCurrentTab$,
this.vaultPopupAutofillService.nonLoginCipherTypesOnPage$,
]).pipe( ]).pipe(
map(([showCards, showIdentities]) => { map(([showCardsSettingEnabled, showIdentitiesSettingEnabled, nonLoginCipherTypesOnPage]) => {
const showCards = showCardsSettingEnabled && nonLoginCipherTypesOnPage[CipherType.Card];
const showIdentities =
showIdentitiesSettingEnabled && nonLoginCipherTypesOnPage[CipherType.Identity];
return [ return [
...(showCards ? [CipherType.Card] : []), ...(showCards ? [CipherType.Card] : []),
...(showIdentities ? [CipherType.Identity] : []), ...(showIdentities ? [CipherType.Identity] : []),