diff --git a/apps/browser/src/autofill/commands/autofill-tab-command.ts b/apps/browser/src/autofill/commands/autofill-tab-command.ts deleted file mode 100644 index 16ce40ff3d..0000000000 --- a/apps/browser/src/autofill/commands/autofill-tab-command.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; - -import AutofillPageDetails from "../models/autofill-page-details"; -import { AutofillService } from "../services/abstractions/autofill.service"; - -export class AutofillTabCommand { - constructor(private autofillService: AutofillService) {} - - async doAutofillTabCommand(tab: chrome.tabs.Tab) { - if (!tab.id) { - throw new Error("Tab does not have an id, cannot complete autofill."); - } - - const details = await this.collectPageDetails(tab.id); - await this.autofillService.doAutoFillOnTab( - [ - { - frameId: 0, - tab: tab, - details: details, - }, - ], - tab, - true, - ); - } - - async doAutofillTabWithCipherCommand(tab: chrome.tabs.Tab, cipher: CipherView) { - if (!tab.id) { - throw new Error("Tab does not have an id, cannot complete autofill."); - } - - const details = await this.collectPageDetails(tab.id); - await this.autofillService.doAutoFill({ - tab: tab, - cipher: cipher, - pageDetails: [ - { - frameId: 0, - tab: tab, - details: details, - }, - ], - skipLastUsed: false, - skipUsernameOnlyFill: false, - onlyEmptyFields: false, - onlyVisibleFields: false, - fillNewPassword: true, - allowTotpAutofill: true, - }); - } - - private async collectPageDetails(tabId: number): Promise { - return new Promise((resolve, reject) => { - chrome.tabs.sendMessage( - tabId, - { - command: "collectPageDetailsImmediately", - }, - (response: AutofillPageDetails) => { - if (chrome.runtime.lastError) { - reject(chrome.runtime.lastError); - return; - } - - resolve(response); - }, - ); - }); - } -} diff --git a/apps/browser/src/autofill/enums/autofill-message.enums.ts b/apps/browser/src/autofill/enums/autofill-message.enums.ts new file mode 100644 index 0000000000..4fdae2d914 --- /dev/null +++ b/apps/browser/src/autofill/enums/autofill-message.enums.ts @@ -0,0 +1,14 @@ +export const AutofillMessageCommand = { + collectPageDetails: "collectPageDetails", + collectPageDetailsResponse: "collectPageDetailsResponse", +} as const; + +export type AutofillMessageCommandType = + (typeof AutofillMessageCommand)[keyof typeof AutofillMessageCommand]; + +export const AutofillMessageSender = { + collectPageDetailsFromTabObservable: "collectPageDetailsFromTabObservable", +} as const; + +export type AutofillMessageSenderType = + (typeof AutofillMessageSender)[keyof typeof AutofillMessageSender]; diff --git a/apps/browser/src/autofill/services/abstractions/autofill.service.ts b/apps/browser/src/autofill/services/abstractions/autofill.service.ts index 54a91a5176..9bdb85a3f2 100644 --- a/apps/browser/src/autofill/services/abstractions/autofill.service.ts +++ b/apps/browser/src/autofill/services/abstractions/autofill.service.ts @@ -1,7 +1,11 @@ +import { Observable } from "rxjs"; + import { UriMatchStrategySetting } from "@bitwarden/common/models/domain/domain-service"; +import { CommandDefinition } from "@bitwarden/common/platform/messaging"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { AutofillMessageCommand } from "../../enums/autofill-message.enums"; import AutofillField from "../../models/autofill-field"; import AutofillForm from "../../models/autofill-form"; import AutofillPageDetails from "../../models/autofill-page-details"; @@ -44,7 +48,20 @@ export interface GenerateFillScriptOptions { defaultUriMatch: UriMatchStrategySetting; } +export type CollectPageDetailsResponseMessage = { + tab: chrome.tabs.Tab; + details: AutofillPageDetails; + sender?: string; + webExtSender: chrome.runtime.MessageSender; +}; + +export const COLLECT_PAGE_DETAILS_RESPONSE_COMMAND = + new CommandDefinition( + AutofillMessageCommand.collectPageDetailsResponse, + ); + export abstract class AutofillService { + collectPageDetailsFromTab$: (tab: chrome.tabs.Tab) => Observable; loadAutofillScriptsOnInstall: () => Promise; reloadAutofillScripts: () => Promise; injectAutofillScripts: ( diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 23f690544d..2427c754ae 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -1,5 +1,5 @@ import { mock, mockReset, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject, of } from "rxjs"; +import { BehaviorSubject, of, Subject } from "rxjs"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -16,12 +16,14 @@ import { EventType } from "@bitwarden/common/enums"; import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service"; import { FakeStateProvider, FakeAccountService, mockAccountServiceWith, + subscribeTo, } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; import { FieldType, LinkedIdType, LoginLinkedId, CipherType } from "@bitwarden/common/vault/enums"; @@ -37,6 +39,7 @@ import { TotpService } from "@bitwarden/common/vault/services/totp.service"; import { BrowserApi } from "../../platform/browser/browser-api"; import { BrowserScriptInjectorService } from "../../platform/services/browser-script-injector.service"; +import { AutofillMessageCommand, AutofillMessageSender } from "../enums/autofill-message.enums"; import { AutofillPort } from "../enums/autofill-port.enums"; import AutofillField from "../models/autofill-field"; import AutofillPageDetails from "../models/autofill-page-details"; @@ -52,6 +55,7 @@ import { flushPromises, triggerTestFailure } from "../spec/testing-utils"; import { AutoFillOptions, + CollectPageDetailsResponseMessage, GenerateFillScriptOptions, PageDetail, } from "./abstractions/autofill.service"; @@ -82,6 +86,7 @@ describe("AutofillService", () => { const platformUtilsService = mock(); let activeAccountStatusMock$: BehaviorSubject; let authService: MockProxy; + let messageListener: MockProxy; beforeEach(() => { scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); @@ -91,6 +96,7 @@ describe("AutofillService", () => { activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked); authService = mock(); authService.activeAccountStatus$ = activeAccountStatusMock$; + messageListener = mock(); autofillService = new AutofillService( cipherService, autofillSettingsService, @@ -103,10 +109,11 @@ describe("AutofillService", () => { scriptInjectorService, accountService, authService, + messageListener, ); - domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider); domainSettingsService.equivalentDomains$ = of(mockEquivalentDomains); + jest.spyOn(BrowserApi, "tabSendMessage"); }); afterEach(() => { @@ -114,6 +121,84 @@ describe("AutofillService", () => { mockReset(cipherService); }); + describe("collectPageDetailsFromTab$", () => { + const tab = mock({ id: 1 }); + const messages = new Subject(); + + function mockCollectPageDetailsResponseMessage( + tab: chrome.tabs.Tab, + webExtSender: chrome.runtime.MessageSender = mock(), + sender: string = AutofillMessageSender.collectPageDetailsFromTabObservable, + ): CollectPageDetailsResponseMessage { + return mock({ + tab, + webExtSender, + sender, + }); + } + + beforeEach(() => { + messageListener.messages$.mockReturnValue(messages.asObservable()); + }); + + it("sends a `collectPageDetails` message to the passed tab", () => { + autofillService.collectPageDetailsFromTab$(tab); + + expect(BrowserApi.tabSendMessage).toHaveBeenCalledWith(tab, { + command: AutofillMessageCommand.collectPageDetails, + sender: AutofillMessageSender.collectPageDetailsFromTabObservable, + tab, + }); + }); + + it("builds an array of page details from received `collectPageDetailsResponse` messages", async () => { + const topLevelSender = mock({ tab, frameId: 0 }); + const subFrameSender = mock({ tab, frameId: 1 }); + + const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab)); + const pausePromise = tracker.pauseUntilReceived(2); + + messages.next(mockCollectPageDetailsResponseMessage(tab, topLevelSender)); + messages.next(mockCollectPageDetailsResponseMessage(tab, subFrameSender)); + + await pausePromise; + + expect(tracker.emissions[1].length).toBe(2); + }); + + it("ignores messages from a different tab", async () => { + const otherTab = mock({ id: 2 }); + + const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab)); + const pausePromise = tracker.pauseUntilReceived(1); + + messages.next(mockCollectPageDetailsResponseMessage(tab)); + messages.next(mockCollectPageDetailsResponseMessage(otherTab)); + + await pausePromise; + + expect(tracker.emissions[1]).toBeUndefined(); + }); + + it("ignores messages from a different sender", async () => { + const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab)); + const pausePromise = tracker.pauseUntilReceived(1); + + messages.next(mockCollectPageDetailsResponseMessage(tab)); + messages.next( + mockCollectPageDetailsResponseMessage( + tab, + mock(), + "some-other-sender", + ), + ); + + await pausePromise; + + expect(tracker.emissions[1]).toBeUndefined(); + }); + }); + describe("loadAutofillScriptsOnInstall", () => { let tab1: chrome.tabs.Tab; let tab2: chrome.tabs.Tab; diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 9ec2052381..ef051e5912 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, startWith } from "rxjs"; +import { filter, firstValueFrom, Observable, scan, startWith } from "rxjs"; import { pairwise } from "rxjs/operators"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -17,6 +17,7 @@ import { UriMatchStrategy, } from "@bitwarden/common/models/domain/domain-service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; import { FieldType, CipherType } from "@bitwarden/common/vault/enums"; @@ -27,6 +28,7 @@ import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; import { BrowserApi } from "../../platform/browser/browser-api"; import { ScriptInjectorService } from "../../platform/services/abstractions/script-injector.service"; import { openVaultItemPasswordRepromptPopout } from "../../vault/popup/utils/vault-popout-window"; +import { AutofillMessageCommand, AutofillMessageSender } from "../enums/autofill-message.enums"; import { AutofillPort } from "../enums/autofill-port.enums"; import AutofillField from "../models/autofill-field"; import AutofillPageDetails from "../models/autofill-page-details"; @@ -35,6 +37,7 @@ import AutofillScript from "../models/autofill-script"; import { AutoFillOptions, AutofillService as AutofillServiceInterface, + COLLECT_PAGE_DETAILS_RESPONSE_COMMAND, FormData, GenerateFillScriptOptions, PageDetail, @@ -64,8 +67,47 @@ export default class AutofillService implements AutofillServiceInterface { private scriptInjectorService: ScriptInjectorService, private accountService: AccountService, private authService: AuthService, + private messageListener: MessageListener, ) {} + /** + * Collects page details from the specific tab. This method returns an observable that can + * be subscribed to in order to build the results from all collectPageDetailsResponse + * messages from the given tab. + * + * @param tab The tab to collect page details from + */ + collectPageDetailsFromTab$(tab: chrome.tabs.Tab): Observable { + const pageDetailsFromTab$ = this.messageListener + .messages$(COLLECT_PAGE_DETAILS_RESPONSE_COMMAND) + .pipe( + filter( + (message) => + message.tab.id === tab.id && + message.sender === AutofillMessageSender.collectPageDetailsFromTabObservable, + ), + scan( + (acc, message) => [ + ...acc, + { + frameId: message.webExtSender.frameId, + tab: message.tab, + details: message.details, + }, + ], + [] as PageDetail[], + ), + ); + + void BrowserApi.tabSendMessage(tab, { + tab: tab, + command: AutofillMessageCommand.collectPageDetails, + sender: AutofillMessageSender.collectPageDetailsFromTabObservable, + }); + + return pageDetailsFromTab$; + } + /** * Triggers on installation of the extension Handles injecting * content scripts into all tabs that are currently open, and diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 9af1a14132..fe797dc8ba 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -889,6 +889,7 @@ export default class MainBackground { this.scriptInjectorService, this.accountService, this.authService, + messageListener, ); this.auditService = new AuditService(this.cryptoFunctionService, this.apiService); diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index d61fa3b19c..c7b5ca9b41 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -342,6 +342,7 @@ const safeProviders: SafeProvider[] = [ ScriptInjectorService, AccountServiceAbstraction, AuthService, + MessageListener, ], }), safeProvider({ diff --git a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts index 24ca030284..97856a952c 100644 --- a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts +++ b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; -import { Subject, firstValueFrom, from } from "rxjs"; +import { Subject, firstValueFrom, from, Subscription } from "rxjs"; import { debounceTime, switchMap, takeUntil } from "rxjs/operators"; import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service"; @@ -51,12 +51,12 @@ export class CurrentTabComponent implements OnInit, OnDestroy { autofillCalloutText: string; protected search$ = new Subject(); private destroy$ = new Subject(); + private collectPageDetailsSubscription: Subscription; private totpCode: string; private totpTimeout: number; private loadedTimeout: number; private searchTimeout: number; - private initPageDetailsTimeout: number; protected unassignedItemsBannerEnabled$ = this.configService.getFeatureFlag$( FeatureFlag.UnassignedItemsBanner, @@ -100,15 +100,6 @@ export class CurrentTabComponent implements OnInit, OnDestroy { }, 500); } break; - case "collectPageDetailsResponse": - if (message.sender === BroadcasterSubscriptionId) { - this.pageDetails.push({ - frameId: message.webExtSender.frameId, - tab: message.tab, - details: message.details, - }); - } - break; default: break; } @@ -266,6 +257,7 @@ export class CurrentTabComponent implements OnInit, OnDestroy { protected async load() { this.isLoading = false; this.tab = await BrowserApi.getTabFromCurrentWindow(); + if (this.tab != null) { this.url = this.tab.url; } else { @@ -274,8 +266,14 @@ export class CurrentTabComponent implements OnInit, OnDestroy { return; } - this.hostname = Utils.getHostname(this.url); this.pageDetails = []; + this.collectPageDetailsSubscription?.unsubscribe(); + this.collectPageDetailsSubscription = this.autofillService + .collectPageDetailsFromTab$(this.tab) + .pipe(takeUntil(this.destroy$)) + .subscribe((pageDetails) => (this.pageDetails = pageDetails)); + + this.hostname = Utils.getHostname(this.url); const otherTypes: CipherType[] = []; const dontShowCards = !(await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$)); const dontShowIdentities = !(await firstValueFrom( @@ -323,7 +321,6 @@ export class CurrentTabComponent implements OnInit, OnDestroy { } this.isLoading = this.loaded = true; - this.collectTabPageDetails(); } async goToSettings() { @@ -361,19 +358,4 @@ export class CurrentTabComponent implements OnInit, OnDestroy { this.autofillCalloutText = this.i18nService.t("autofillSelectInfoWithoutCommand"); } } - - private collectTabPageDetails() { - void BrowserApi.tabSendMessage(this.tab, { - command: "collectPageDetails", - tab: this.tab, - sender: BroadcasterSubscriptionId, - }); - - window.clearTimeout(this.initPageDetailsTimeout); - this.initPageDetailsTimeout = window.setTimeout(() => { - if (this.pageDetails.length === 0) { - this.collectTabPageDetails(); - } - }, 250); - } } diff --git a/apps/browser/src/vault/popup/components/vault/view.component.ts b/apps/browser/src/vault/popup/components/vault/view.component.ts index 211bd8fc09..ba101aa653 100644 --- a/apps/browser/src/vault/popup/components/vault/view.component.ts +++ b/apps/browser/src/vault/popup/components/vault/view.component.ts @@ -1,7 +1,7 @@ import { DatePipe, Location } from "@angular/common"; import { ChangeDetectorRef, Component, NgZone } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { Subject, firstValueFrom, takeUntil } from "rxjs"; +import { Subject, firstValueFrom, takeUntil, Subscription } from "rxjs"; import { first } from "rxjs/operators"; import { ViewComponent as BaseViewComponent } from "@bitwarden/angular/vault/components/view.component"; @@ -68,6 +68,7 @@ export class ViewComponent extends BaseViewComponent { inPopout = false; cipherType = CipherType; private fido2PopoutSessionData$ = fido2PopoutSessionData$(); + private collectPageDetailsSubscription: Subscription; private destroy$ = new Subject(); @@ -152,15 +153,6 @@ export class ViewComponent extends BaseViewComponent { // eslint-disable-next-line @typescript-eslint/no-floating-promises this.ngZone.run(async () => { switch (message.command) { - case "collectPageDetailsResponse": - if (message.sender === BroadcasterSubscriptionId) { - this.pageDetails.push({ - frameId: message.webExtSender.frameId, - tab: message.tab, - details: message.details, - }); - } - break; case "tabChanged": case "windowChanged": if (this.loadPageDetailsTimeout != null) { @@ -337,6 +329,7 @@ export class ViewComponent extends BaseViewComponent { } private async loadPageDetails() { + this.collectPageDetailsSubscription?.unsubscribe(); this.pageDetails = []; this.tab = this.senderTabId ? await BrowserApi.getTab(this.senderTabId) @@ -346,13 +339,10 @@ export class ViewComponent extends BaseViewComponent { return; } - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - BrowserApi.tabSendMessage(this.tab, { - command: "collectPageDetails", - tab: this.tab, - sender: BroadcasterSubscriptionId, - }); + this.collectPageDetailsSubscription = this.autofillService + .collectPageDetailsFromTab$(this.tab) + .pipe(takeUntil(this.destroy$)) + .subscribe((pageDetails) => (this.pageDetails = pageDetails)); } private async doAutofill() {