From 696c53fac7fea2cd977155d0b1688277188c83e1 Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Mon, 29 Dec 2025 16:41:42 -0800 Subject: [PATCH 1/7] [PM-29209] Fix persistent browser settings berry (#18113) * [PM-29209] Introduce new autofill nudge service specific to the Browser client * [PM-29209] Cleanup redundant browser setting checks * [PM-29209] Ensure nudge is dismissed on nudge button click * [PM-29209] Add spec file for browser autofill nudge service * [PM-29209] Cleanup settings-v2 spec file --- .../popup/settings/autofill.component.html | 22 +-- .../popup/settings/autofill.component.ts | 4 + .../src/popup/services/services.module.ts | 8 + .../popup/settings/settings-v2.component.html | 12 +- .../settings/settings-v2.component.spec.ts | 46 +---- .../popup/settings/settings-v2.component.ts | 33 +--- .../browser-autofill-nudge.service.spec.ts | 157 ++++++++++++++++++ .../browser-autofill-nudge.service.ts | 37 +++++ libs/angular/src/vault/index.ts | 1 + .../services/custom-nudges-services/index.ts | 1 + .../noop-nudge.service.ts | 27 +++ .../vault/services/nudge-injection-tokens.ts | 7 + .../src/vault/services/nudges.service.ts | 10 +- 13 files changed, 272 insertions(+), 93 deletions(-) create mode 100644 apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts create mode 100644 apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts create mode 100644 libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts create mode 100644 libs/angular/src/vault/services/nudge-injection-tokens.ts diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.html b/apps/browser/src/autofill/popup/settings/autofill.component.html index 1153ad58719..085145adb19 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.html +++ b/apps/browser/src/autofill/popup/settings/autofill.component.html @@ -6,16 +6,18 @@
-
- -
+ @if (showSpotlightNudge$ | async) { +
+ +
+ }

{{ "autofillSuggestionsSectionTitle" | i18n }}

diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index 49be3104dc1..acb2aa7a970 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -611,6 +611,10 @@ export class AutofillComponent implements OnInit { if (this.canOverrideBrowserAutofillSetting) { this.defaultBrowserAutofillDisabled = true; await this.updateDefaultBrowserAutofillDisabled(); + await this.nudgesService.dismissNudge( + NudgeType.AutofillNudge, + await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)), + ); } else { await this.openURI(event, this.disablePasswordManagerURI); } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 739166ff6f8..7a2ded5bb83 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -23,6 +23,8 @@ import { WINDOW, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; +import { AUTOFILL_NUDGE_SERVICE } from "@bitwarden/angular/vault"; +import { SingleNudgeService } from "@bitwarden/angular/vault/services/default-single-nudge.service"; import { LoginComponentService, TwoFactorAuthComponentService, @@ -208,6 +210,7 @@ import { } from "../../platform/system-notifications/browser-system-notification.service"; import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging"; import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service"; +import { BrowserAutofillNudgeService } from "../../vault/popup/services/browser-autofill-nudge.service"; import { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service"; import { ExtensionAnonLayoutWrapperDataService } from "../components/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service"; @@ -756,6 +759,11 @@ const safeProviders: SafeProvider[] = [ MessagingServiceAbstraction, ], }), + safeProvider({ + provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken, + useClass: BrowserAutofillNudgeService, + deps: [], + }), ]; @NgModule({ diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.html b/apps/browser/src/tools/popup/settings/settings-v2.component.html index 683b7d70ed6..06c89e15f59 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.html +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.html @@ -34,13 +34,11 @@

{{ "autofill" | i18n }}

- 1 + @if (showAutofillBadge$ | async) { + 1 + }
diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts b/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts index f51d514289e..4cc3ed0149c 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts @@ -148,31 +148,7 @@ describe("SettingsV2Component", () => { expect(openSpy).toHaveBeenCalledWith(dialogService); }); - it("isBrowserAutofillSettingOverridden$ emits the value from the AutofillBrowserSettingsService", async () => { - pushActiveAccount(); - - mockAutofillSettings.isBrowserAutofillSettingOverridden.mockResolvedValue(true); - - const fixture = TestBed.createComponent(SettingsV2Component); - const component = fixture.componentInstance; - fixture.detectChanges(); - await fixture.whenStable(); - - const value = await firstValueFrom(component["isBrowserAutofillSettingOverridden$"]); - expect(value).toBe(true); - - mockAutofillSettings.isBrowserAutofillSettingOverridden.mockResolvedValue(false); - - const fixture2 = TestBed.createComponent(SettingsV2Component); - const component2 = fixture2.componentInstance; - fixture2.detectChanges(); - await fixture2.whenStable(); - - const value2 = await firstValueFrom(component2["isBrowserAutofillSettingOverridden$"]); - expect(value2).toBe(false); - }); - - it("showAutofillBadge$ emits true when default autofill is NOT disabled and nudge is true", async () => { + it("showAutofillBadge$ emits true when showNudgeBadge is true", async () => { pushActiveAccount(); mockNudges.showNudgeBadge$.mockImplementation((type: NudgeType) => @@ -184,30 +160,10 @@ describe("SettingsV2Component", () => { fixture.detectChanges(); await fixture.whenStable(); - mockAutofillSettings.defaultBrowserAutofillDisabled$.next(false); - const value = await firstValueFrom(component.showAutofillBadge$); expect(value).toBe(true); }); - it("showAutofillBadge$ emits false when default autofill IS disabled even if nudge is true", async () => { - pushActiveAccount(); - - mockNudges.showNudgeBadge$.mockImplementation((type: NudgeType) => - of(type === NudgeType.AutofillNudge), - ); - - const fixture = TestBed.createComponent(SettingsV2Component); - const component = fixture.componentInstance; - fixture.detectChanges(); - await fixture.whenStable(); - - mockAutofillSettings.defaultBrowserAutofillDisabled$.next(true); - - const value = await firstValueFrom(component.showAutofillBadge$); - expect(value).toBe(false); - }); - it("dismissBadge dismisses when showVaultBadge$ emits true", async () => { const acct = pushActiveAccount(); diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.ts b/apps/browser/src/tools/popup/settings/settings-v2.component.ts index 95aeeb2f480..e10d41b9445 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.ts +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.ts @@ -1,16 +1,7 @@ import { CommonModule } from "@angular/common"; import { ChangeDetectionStrategy, Component } from "@angular/core"; import { RouterModule } from "@angular/router"; -import { - combineLatest, - filter, - firstValueFrom, - from, - map, - Observable, - shareReplay, - switchMap, -} from "rxjs"; +import { filter, firstValueFrom, Observable, shareReplay, switchMap } from "rxjs"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -28,8 +19,6 @@ import { } from "@bitwarden/components"; import { CurrentAccountComponent } from "../../../auth/popup/account-switching/current-account.component"; -import { AutofillBrowserSettingsService } from "../../../autofill/services/autofill-browser-settings.service"; -import { BrowserApi } from "../../../platform/browser/browser-api"; import { PopOutComponent } from "../../../platform/popup/components/pop-out.component"; import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; @@ -55,12 +44,6 @@ import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.co export class SettingsV2Component { NudgeType = NudgeType; - protected isBrowserAutofillSettingOverridden$ = from( - this.autofillBrowserSettingsService.isBrowserAutofillSettingOverridden( - BrowserApi.getBrowserClientVendor(window), - ), - ); - private authenticatedAccount$: Observable = this.accountService.activeAccount$.pipe( filter((account): account is Account => account !== null), shareReplay({ bufferSize: 1, refCount: true }), @@ -82,23 +65,13 @@ export class SettingsV2Component { ), ); - showAutofillBadge$: Observable = combineLatest([ - this.autofillBrowserSettingsService.defaultBrowserAutofillDisabled$, - this.authenticatedAccount$, - ]).pipe( - switchMap(([defaultBrowserAutofillDisabled, account]) => - this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id).pipe( - map((badgeStatus) => { - return !defaultBrowserAutofillDisabled && badgeStatus; - }), - ), - ), + showAutofillBadge$: Observable = this.authenticatedAccount$.pipe( + switchMap((account) => this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id)), ); constructor( private readonly nudgesService: NudgesService, private readonly accountService: AccountService, - private readonly autofillBrowserSettingsService: AutofillBrowserSettingsService, private readonly accountProfileStateService: BillingAccountProfileStateService, private readonly dialogService: DialogService, ) {} diff --git a/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts new file mode 100644 index 00000000000..40782760283 --- /dev/null +++ b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts @@ -0,0 +1,157 @@ +import { TestBed } from "@angular/core/testing"; +import { mock, MockProxy } from "jest-mock-extended"; +import { firstValueFrom } from "rxjs"; + +import { NudgeStatus, NudgeType } from "@bitwarden/angular/vault"; +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; +import { BrowserClientVendors } from "@bitwarden/common/autofill/constants"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { StateProvider } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { FakeStateProvider, mockAccountServiceWith } from "../../../../../../libs/common/spec"; +import { BrowserApi } from "../../../platform/browser/browser-api"; + +import { BrowserAutofillNudgeService } from "./browser-autofill-nudge.service"; + +describe("BrowserAutofillNudgeService", () => { + let service: BrowserAutofillNudgeService; + let vaultProfileService: MockProxy; + let fakeStateProvider: FakeStateProvider; + + const userId = "test-user-id" as UserId; + const nudgeType = NudgeType.AutofillNudge; + + const notDismissedStatus: NudgeStatus = { + hasBadgeDismissed: false, + hasSpotlightDismissed: false, + }; + + const dismissedStatus: NudgeStatus = { + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }; + + // Set profile creation date to now (new account, within 30 days) + const recentProfileDate = new Date(); + + beforeEach(() => { + vaultProfileService = mock(); + vaultProfileService.getProfileCreationDate.mockResolvedValue(recentProfileDate); + + fakeStateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); + + TestBed.configureTestingModule({ + providers: [ + BrowserAutofillNudgeService, + { + provide: VaultProfileService, + useValue: vaultProfileService, + }, + { + provide: StateProvider, + useValue: fakeStateProvider, + }, + { + provide: LogService, + useValue: mock(), + }, + ], + }); + + service = TestBed.inject(BrowserAutofillNudgeService); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe("nudgeStatus$", () => { + it("returns parent status when browser client is Unknown", async () => { + jest + .spyOn(BrowserApi, "getBrowserClientVendor") + .mockReturnValue(BrowserClientVendors.Unknown); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(notDismissedStatus); + }); + + it("returns parent status when browser autofill is not overridden", async () => { + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(false); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(notDismissedStatus); + }); + + it("returns dismissed status when browser autofill is overridden", async () => { + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(dismissedStatus); + }); + + it("preserves parent dismissed status when account is older than 30 days", async () => { + // Set profile creation date to more than 30 days ago + const oldProfileDate = new Date(Date.now() - 31 * 24 * 60 * 60 * 1000); + vaultProfileService.getProfileCreationDate.mockResolvedValue(oldProfileDate); + + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(false); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(dismissedStatus); + }); + + it("combines parent dismissed and browser autofill overridden status", async () => { + // Set profile creation date to more than 30 days ago (parent dismisses) + const oldProfileDate = new Date(Date.now() - 31 * 24 * 60 * 60 * 1000); + vaultProfileService.getProfileCreationDate.mockResolvedValue(oldProfileDate); + + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(dismissedStatus); + }); + + it.each([ + BrowserClientVendors.Chrome, + BrowserClientVendors.Edge, + BrowserClientVendors.Opera, + BrowserClientVendors.Vivaldi, + ])("checks browser autofill settings for %s browser", async (browserVendor) => { + const getBrowserClientVendorSpy = jest + .spyOn(BrowserApi, "getBrowserClientVendor") + .mockReturnValue(browserVendor); + const browserAutofillSettingsOverriddenSpy = jest + .spyOn(BrowserApi, "browserAutofillSettingsOverridden") + .mockResolvedValue(true); + + await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(getBrowserClientVendorSpy).toHaveBeenCalledWith(window); + expect(browserAutofillSettingsOverriddenSpy).toHaveBeenCalled(); + }); + + it("does not check browser autofill settings for Unknown browser", async () => { + jest + .spyOn(BrowserApi, "getBrowserClientVendor") + .mockReturnValue(BrowserClientVendors.Unknown); + const browserAutofillSettingsOverriddenSpy = jest + .spyOn(BrowserApi, "browserAutofillSettingsOverridden") + .mockResolvedValue(true); + + await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(browserAutofillSettingsOverriddenSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts new file mode 100644 index 00000000000..7fe5f527bcb --- /dev/null +++ b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts @@ -0,0 +1,37 @@ +import { Injectable } from "@angular/core"; +import { Observable, switchMap } from "rxjs"; + +import { NudgeStatus, NudgeType } from "@bitwarden/angular/vault"; +import { NewAccountNudgeService } from "@bitwarden/angular/vault/services/custom-nudges-services/new-account-nudge.service"; +import { BrowserClientVendors } from "@bitwarden/common/autofill/constants"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { BrowserApi } from "../../../platform/browser/browser-api"; + +/** + * Browser-specific autofill nudge service. + * Extends NewAccountNudgeService (30-day account age check) and adds + * browser autofill setting detection. + * + * Nudge is dismissed if: + * - Account is older than 30 days (inherited from NewAccountNudgeService) + * - Browser's built-in password manager is already disabled via privacy settings + */ +@Injectable() +export class BrowserAutofillNudgeService extends NewAccountNudgeService { + override nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { + return super.nudgeStatus$(nudgeType, userId).pipe( + switchMap(async (status) => { + const browserClient = BrowserApi.getBrowserClientVendor(window); + const browserAutofillOverridden = + browserClient !== BrowserClientVendors.Unknown && + (await BrowserApi.browserAutofillSettingsOverridden()); + + return { + hasBadgeDismissed: status.hasBadgeDismissed || browserAutofillOverridden, + hasSpotlightDismissed: status.hasSpotlightDismissed || browserAutofillOverridden, + }; + }), + ); + } +} diff --git a/libs/angular/src/vault/index.ts b/libs/angular/src/vault/index.ts index cb43fadb3bc..b9131338a45 100644 --- a/libs/angular/src/vault/index.ts +++ b/libs/angular/src/vault/index.ts @@ -1,3 +1,4 @@ // Note: Nudge related code is exported from `libs/angular` because it is consumed by multiple // `libs/*` packages. Exporting from the `libs/vault` package creates circular dependencies. export { NudgesService, NudgeStatus, NudgeType } from "./services/nudges.service"; +export { AUTOFILL_NUDGE_SERVICE } from "./services/nudge-injection-tokens"; diff --git a/libs/angular/src/vault/services/custom-nudges-services/index.ts b/libs/angular/src/vault/services/custom-nudges-services/index.ts index f60592b9c71..d4bfe80a525 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/index.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/index.ts @@ -4,3 +4,4 @@ export * from "./empty-vault-nudge.service"; export * from "./vault-settings-import-nudge.service"; export * from "./new-item-nudge.service"; export * from "./new-account-nudge.service"; +export * from "./noop-nudge.service"; diff --git a/libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts new file mode 100644 index 00000000000..eabf1d6fc4a --- /dev/null +++ b/libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts @@ -0,0 +1,27 @@ +import { Injectable } from "@angular/core"; +import { Observable, of } from "rxjs"; + +import { UserId } from "@bitwarden/common/types/guid"; + +import { SingleNudgeService } from "../default-single-nudge.service"; +import { NudgeStatus, NudgeType } from "../nudges.service"; + +/** + * A no-op nudge service that always returns dismissed status. + * Use this for nudges that should be completely ignored/hidden in certain clients. + * For example, browser-specific nudges can use this as the default in non-browser clients. + */ +@Injectable({ providedIn: "root" }) +export class NoOpNudgeService implements SingleNudgeService { + nudgeStatus$(_nudgeType: NudgeType, _userId: UserId): Observable { + return of({ hasBadgeDismissed: true, hasSpotlightDismissed: true }); + } + + async setNudgeStatus( + _nudgeType: NudgeType, + _newStatus: NudgeStatus, + _userId: UserId, + ): Promise { + // No-op: state changes are ignored + } +} diff --git a/libs/angular/src/vault/services/nudge-injection-tokens.ts b/libs/angular/src/vault/services/nudge-injection-tokens.ts new file mode 100644 index 00000000000..52a0838d356 --- /dev/null +++ b/libs/angular/src/vault/services/nudge-injection-tokens.ts @@ -0,0 +1,7 @@ +import { InjectionToken } from "@angular/core"; + +import { SingleNudgeService } from "./default-single-nudge.service"; + +export const AUTOFILL_NUDGE_SERVICE = new InjectionToken( + "AutofillNudgeService", +); diff --git a/libs/angular/src/vault/services/nudges.service.ts b/libs/angular/src/vault/services/nudges.service.ts index 05d565eb499..19acf690d32 100644 --- a/libs/angular/src/vault/services/nudges.service.ts +++ b/libs/angular/src/vault/services/nudges.service.ts @@ -12,8 +12,10 @@ import { NewItemNudgeService, AccountSecurityNudgeService, VaultSettingsImportNudgeService, + NoOpNudgeService, } from "./custom-nudges-services"; import { DefaultSingleNudgeService, SingleNudgeService } from "./default-single-nudge.service"; +import { AUTOFILL_NUDGE_SERVICE } from "./nudge-injection-tokens"; export type NudgeStatus = { hasBadgeDismissed: boolean; @@ -56,6 +58,12 @@ export class NudgesService { private newItemNudgeService = inject(NewItemNudgeService); private newAcctNudgeService = inject(NewAccountNudgeService); + // NoOp service that always returns dismissed + private noOpNudgeService = inject(NoOpNudgeService); + + // Optional Browser-specific service provided via injection token (not all clients have autofill) + private autofillNudgeService = inject(AUTOFILL_NUDGE_SERVICE, { optional: true }); + /** * Custom nudge services to use for specific nudge types * Each nudge type can have its own service to determine when to show the nudge @@ -66,7 +74,7 @@ export class NudgesService { [NudgeType.EmptyVaultNudge]: inject(EmptyVaultNudgeService), [NudgeType.VaultSettingsImportNudge]: inject(VaultSettingsImportNudgeService), [NudgeType.AccountSecurity]: inject(AccountSecurityNudgeService), - [NudgeType.AutofillNudge]: this.newAcctNudgeService, + [NudgeType.AutofillNudge]: this.autofillNudgeService ?? this.noOpNudgeService, [NudgeType.DownloadBitwarden]: this.newAcctNudgeService, [NudgeType.GeneratorNudgeStatus]: this.newAcctNudgeService, [NudgeType.NewLoginItemStatus]: this.newItemNudgeService, From cf1c3226c3f0d9002ec6454330e7f8d1366941c3 Mon Sep 17 00:00:00 2001 From: aj-bw <81774843+aj-bw@users.noreply.github.com> Date: Tue, 30 Dec 2025 09:07:32 +0000 Subject: [PATCH 2/7] replace inline removal with reusable workflow (#18144) --- .github/workflows/build-desktop.yml | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index f3cdf80f710..45297a110a0 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -179,18 +179,8 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false - - name: Free disk space for build - run: | - sudo rm -rf /usr/share/dotnet - sudo rm -rf /usr/share/swift - sudo rm -rf /usr/local/.ghcup - sudo rm -rf /usr/share/miniconda - sudo rm -rf /usr/share/az_* - sudo rm -rf /usr/local/julia* - sudo rm -rf /usr/lib/mono - sudo rm -rf /usr/lib/heroku - sudo rm -rf /usr/local/aws-cli - sudo rm -rf /usr/local/aws-sam-cli + - name: Free disk space + uses: bitwarden/gh-actions/free-disk-space@main - name: Set up Node uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 From 8a6f9bfaeb84aa07b6c4d5cdd61305e4373d9b93 Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Tue, 30 Dec 2025 10:36:08 -0500 Subject: [PATCH 3/7] [PM-29515] Remove ts strict ignore in overlay inline menu iframe content autofill inline menu iframe service (#18030) * use optional chaining and make portkey optional to match the AutofillInlineMenuIframeExtensionMessage * make ariaAlertElement optional * tiemouts are set to null for clearing, updated type to match this * border color is conditionally applied, undefined is acceptable here * check if aria alerts exist before calling * return early if no styles exist for updateElementStyles or no position for updateIframePosition * initilaize timers to null * non null assert iframe since it is initialized in initMenuIframe which makes it safe to assert non null by lifecycle * remove optional chainning --- .../autofill-inline-menu-iframe.service.ts | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts index 64ef7d180ed..ad1241e98d2 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { EVENTS } from "@bitwarden/common/autofill/constants"; import { ThemeTypes } from "@bitwarden/common/platform/enums"; @@ -15,14 +13,17 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe private readonly setElementStyles = setElementStyles; private readonly sendExtensionMessage = sendExtensionMessage; private port: chrome.runtime.Port | null = null; - private portKey: string; + private portKey?: string; private readonly extensionOrigin: string; private iframeMutationObserver: MutationObserver; - private iframe: HTMLIFrameElement; - private ariaAlertElement: HTMLDivElement; - private ariaAlertTimeout: number | NodeJS.Timeout; - private delayedCloseTimeout: number | NodeJS.Timeout; - private fadeInTimeout: number | NodeJS.Timeout; + /** + * Initialized in initMenuIframe which makes it safe to assert non null by lifecycle. + */ + private iframe!: HTMLIFrameElement; + private ariaAlertElement?: HTMLDivElement; + private ariaAlertTimeout: number | NodeJS.Timeout | null = null; + private delayedCloseTimeout: number | NodeJS.Timeout | null = null; + private fadeInTimeout: number | NodeJS.Timeout | null = null; private readonly fadeInOpacityTransition = "opacity 125ms ease-out 0s"; private readonly fadeOutOpacityTransition = "opacity 65ms ease-out 0s"; private iframeStyles: Partial = { @@ -50,7 +51,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe }; private foreignMutationsCount = 0; private mutationObserverIterations = 0; - private mutationObserverIterationsResetTimeout: number | NodeJS.Timeout; + private mutationObserverIterationsResetTimeout: number | NodeJS.Timeout | null = null; private readonly backgroundPortMessageHandlers: BackgroundPortMessageHandlers = { initAutofillInlineMenuButton: ({ message }) => this.initAutofillInlineMenu(message), initAutofillInlineMenuList: ({ message }) => this.initAutofillInlineMenu(message), @@ -134,7 +135,9 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe this.port.onDisconnect.addListener(this.handlePortDisconnect); this.port.onMessage.addListener(this.handlePortMessage); - this.announceAriaAlert(this.ariaAlert, 2000); + if (this.ariaAlert) { + this.announceAriaAlert(this.ariaAlert, 2000); + } }; /** @@ -155,7 +158,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe this.ariaAlertTimeout = globalThis.setTimeout(async () => { const isFieldFocused = await this.sendExtensionMessage("checkIsFieldCurrentlyFocused"); - if (isFieldFocused || triggeredByUser) { + if ((isFieldFocused || triggeredByUser) && this.ariaAlertElement) { this.shadow.appendChild(this.ariaAlertElement); } this.ariaAlertTimeout = null; @@ -242,7 +245,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe */ private initAutofillInlineMenuList(message: AutofillInlineMenuIframeExtensionMessage) { const { theme } = message; - let borderColor: string; + let borderColor: string | undefined; let verifiedTheme = theme; if (verifiedTheme === ThemeTypes.System) { verifiedTheme = globalThis.matchMedia("(prefers-color-scheme: dark)").matches @@ -274,8 +277,8 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe * * @param position - The position styles to apply to the iframe */ - private updateIframePosition(position: Partial) { - if (!globalThis.document.hasFocus()) { + private updateIframePosition(position?: Partial) { + if (!position || !globalThis.document.hasFocus()) { return; } @@ -295,7 +298,9 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe this.handleFadeInInlineMenuIframe(); } - this.announceAriaAlert(this.ariaAlert, 2000); + if (this.ariaAlert) { + this.announceAriaAlert(this.ariaAlert, 2000); + } } /** @@ -359,8 +364,8 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe * @param customElement - The element to update the styles for * @param styles - The styles to apply to the element */ - private updateElementStyles(customElement: HTMLElement, styles: Partial) { - if (!customElement) { + private updateElementStyles(customElement: HTMLElement, styles?: Partial) { + if (!customElement || !styles) { return; } From cee69f85c0da401c6ec572593e15b536cc6d78e9 Mon Sep 17 00:00:00 2001 From: Ben Brooks <56796209+bensbits91@users.noreply.github.com> Date: Tue, 30 Dec 2025 08:21:10 -0800 Subject: [PATCH 4/7] [pm-28077] Add input types to ignoredInputTypes (#17870) * [pm-28077] Add input types to ignoredInputTypes Signed-off-by: Ben Brooks * Merge branch 'main' of github.com:bitwarden/clients into pm-28077-more-ignoredInputTypes-in-CollectAutofillContentService Signed-off-by: Ben Brooks * [pm-28077] Remove month input type from ignored types Signed-off-by: Ben Brooks * [pm-28077] Remove month radio and checkbox types from ignored types Signed-off-by: Ben Brooks * Merge branch 'main' of github.com:bitwarden/clients into pm-28077-more-ignoredInputTypes-in-CollectAutofillContentService Signed-off-by: Ben Brooks * [pm-28077] Fix prettier issues/conflicts Signed-off-by: Ben Brooks * [pm-28077] Add comment regarding datetime depcrecation Signed-off-by: Ben Brooks --------- Signed-off-by: Ben Brooks --- .../services/collect-autofill-content.service.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 367599f7ad0..18eb8e2baf8 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -60,6 +60,15 @@ export class CollectAutofillContentService implements CollectAutofillContentServ "button", "image", "file", + "search", + "url", + "date", + "time", + "datetime", // Note: datetime is deprecated in HTML5; keeping here for backwards compatibility + "datetime-local", + "week", + "color", + "range", ]); constructor( From 800a21d8a3b65369528610ac79517e7a5792e48a Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Tue, 30 Dec 2025 11:06:30 -0600 Subject: [PATCH 5/7] [PM-28548] Phishing Blocker support links (#18070) * Change domain terminology to web addresses * Added phishing resource file * Finish renaming and adding runtime configuration for domains vs links setting * Update reference * Add matching functions per resource * correct URL matching logic for links-based detection Problem: The phishing link matcher was failing to detect known phishing URLs due to two issues: 1. Protocol mismatch: Entries in the phishing list use `http://` but users typically visit `https://` versions. The matcher was comparing full URLs including protocol, causing legitimate matches to fail. - List entry: `http://smartdapptradxx.pages.dev` - User visits: `https://smartdapptradxx.pages.dev/` - Result: No match (incorrect) 2. Hostname-only matching would have caused false positives: An earlier attempt to fix #1 included hostname-only comparison, which defeats the purpose of links-based detection. The goal of PM-28548 is precise URL matching to avoid blocking entire domains (like pages.dev, github.io) when only specific paths are malicious. Solution: - Always strip protocol (http:// or https://) from both entry and URL before comparison, treating them as equivalent - Remove hostname-only matching to maintain precision - Keep prefix matching for subpaths, query strings, and fragments --------- Co-authored-by: Alex --- .../phishing-detection/phishing-resources.ts | 98 ++++++++++++++++ .../services/phishing-data.service.spec.ts | 62 +++++----- .../services/phishing-data.service.ts | 107 +++++++++--------- .../services/phishing-detection.service.ts | 2 +- 4 files changed, 186 insertions(+), 83 deletions(-) create mode 100644 apps/browser/src/dirt/phishing-detection/phishing-resources.ts diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts new file mode 100644 index 00000000000..262d6cf833b --- /dev/null +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -0,0 +1,98 @@ +export type PhishingResource = { + name?: string; + remoteUrl: string; + checksumUrl: string; + todayUrl: string; + /** Matcher used to decide whether a given URL matches an entry from this resource */ + match: (url: URL, entry: string) => boolean; +}; + +export const PhishingResourceType = Object.freeze({ + Domains: "domains", + Links: "links", +} as const); + +export type PhishingResourceType = (typeof PhishingResourceType)[keyof typeof PhishingResourceType]; + +export const PHISHING_RESOURCES: Record = { + [PhishingResourceType.Domains]: [ + { + name: "Phishing.Database Domains", + remoteUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/master/phishing-domains-ACTIVE.txt", + checksumUrl: + "https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-domains-ACTIVE.txt.md5", + todayUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-domains-NEW-today.txt", + match: (url: URL, entry: string) => { + if (!entry) { + return false; + } + const candidate = entry.trim().toLowerCase().replace(/\/$/, ""); + // If entry contains a scheme, strip it for comparison + const e = candidate.replace(/^https?:\/\//, ""); + // Compare against hostname or host+path + if (e === url.hostname.toLowerCase()) { + return true; + } + const urlNoProto = url.href + .toLowerCase() + .replace(/https?:\/\//, "") + .replace(/\/$/, ""); + return urlNoProto === e || urlNoProto.startsWith(e + "/"); + }, + }, + ], + [PhishingResourceType.Links]: [ + { + name: "Phishing.Database Links", + remoteUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/master/phishing-links-ACTIVE.txt", + checksumUrl: + "https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-links-ACTIVE.txt.md5", + todayUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-links-NEW-today.txt", + match: (url: URL, entry: string) => { + if (!entry) { + return false; + } + // Basic HTML entity decode for common cases (the lists sometimes contain &) + const decodeHtml = (s: string) => s.replace(/&/g, "&"); + + const normalizedEntry = decodeHtml(entry.trim()).toLowerCase().replace(/\/$/, ""); + + // Normalize URL for comparison - always strip protocol for consistent matching + const normalizedUrl = decodeHtml(url.href).toLowerCase().replace(/\/$/, ""); + const urlNoProto = normalizedUrl.replace(/^https?:\/\//, ""); + + // Strip protocol from entry if present (http:// and https:// should be treated as equivalent) + const entryNoProto = normalizedEntry.replace(/^https?:\/\//, ""); + + // Compare full path (without protocol) - exact match + if (urlNoProto === entryNoProto) { + return true; + } + + // Check if URL starts with entry (prefix match for subpaths/query/hash) + // e.g., entry "site.com/phish" matches "site.com/phish/subpage" or "site.com/phish?id=1" + if ( + urlNoProto.startsWith(entryNoProto + "/") || + urlNoProto.startsWith(entryNoProto + "?") || + urlNoProto.startsWith(entryNoProto + "#") + ) { + return true; + } + + return false; + }, + }, + ], +}; + +export function getPhishingResources( + type: PhishingResourceType, + index = 0, +): PhishingResource | undefined { + const list = PHISHING_RESOURCES[type] ?? []; + return list[index]; +} diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts index 94f3e99f8be..30aa947092d 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts @@ -25,7 +25,7 @@ describe("PhishingDataService", () => { }; let fetchChecksumSpy: jest.SpyInstance; - let fetchDomainsSpy: jest.SpyInstance; + let fetchWebAddressesSpy: jest.SpyInstance; beforeEach(() => { jest.useFakeTimers(); @@ -45,113 +45,113 @@ describe("PhishingDataService", () => { platformUtilsService, ); - fetchChecksumSpy = jest.spyOn(service as any, "fetchPhishingDomainsChecksum"); - fetchDomainsSpy = jest.spyOn(service as any, "fetchPhishingDomains"); + fetchChecksumSpy = jest.spyOn(service as any, "fetchPhishingChecksum"); + fetchWebAddressesSpy = jest.spyOn(service as any, "fetchPhishingWebAddresses"); }); - describe("isPhishingDomains", () => { - it("should detect a phishing domain", async () => { + describe("isPhishingWebAddress", () => { + it("should detect a phishing web address", async () => { setMockState({ - domains: ["phish.com", "badguy.net"], + webAddresses: ["phish.com", "badguy.net"], timestamp: Date.now(), checksum: "abc123", applicationVersion: "1.0.0", }); const url = new URL("http://phish.com"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(true); }); - it("should not detect a safe domain", async () => { + it("should not detect a safe web address", async () => { setMockState({ - domains: ["phish.com", "badguy.net"], + webAddresses: ["phish.com", "badguy.net"], timestamp: Date.now(), checksum: "abc123", applicationVersion: "1.0.0", }); const url = new URL("http://safe.com"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); }); - it("should match against root domain", async () => { + it("should match against root web address", async () => { setMockState({ - domains: ["phish.com", "badguy.net"], + webAddresses: ["phish.com", "badguy.net"], timestamp: Date.now(), checksum: "abc123", applicationVersion: "1.0.0", }); const url = new URL("http://phish.com/about"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(true); }); it("should not error on empty state", async () => { setMockState(undefined as any); const url = new URL("http://phish.com/about"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); }); }); - describe("getNextDomains", () => { - it("refetches all domains if applicationVersion has changed", async () => { + describe("getNextWebAddresses", () => { + it("refetches all web addresses if applicationVersion has changed", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 60000, checksum: "old", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("new"); - fetchDomainsSpy.mockResolvedValue(["d.com", "e.com"]); + fetchWebAddressesSpy.mockResolvedValue(["d.com", "e.com"]); platformUtilsService.getApplicationVersion.mockResolvedValue("2.0.0"); - const result = await service.getNextDomains(prev); + const result = await service.getNextWebAddresses(prev); - expect(result!.domains).toEqual(["d.com", "e.com"]); + expect(result!.webAddresses).toEqual(["d.com", "e.com"]); expect(result!.checksum).toBe("new"); expect(result!.applicationVersion).toBe("2.0.0"); }); it("only updates timestamp if checksum matches", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 60000, checksum: "abc", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("abc"); - const result = await service.getNextDomains(prev); - expect(result!.domains).toEqual(prev.domains); + const result = await service.getNextWebAddresses(prev); + expect(result!.webAddresses).toEqual(prev.webAddresses); expect(result!.checksum).toBe("abc"); expect(result!.timestamp).not.toBe(prev.timestamp); }); it("patches daily domains if cache is fresh", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 60000, checksum: "old", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("new"); - fetchDomainsSpy.mockResolvedValue(["b.com", "c.com"]); - const result = await service.getNextDomains(prev); - expect(result!.domains).toEqual(["a.com", "b.com", "c.com"]); + fetchWebAddressesSpy.mockResolvedValue(["b.com", "c.com"]); + const result = await service.getNextWebAddresses(prev); + expect(result!.webAddresses).toEqual(["a.com", "b.com", "c.com"]); expect(result!.checksum).toBe("new"); }); it("fetches all domains if cache is old", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 2 * 24 * 60 * 60 * 1000, checksum: "old", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("new"); - fetchDomainsSpy.mockResolvedValue(["d.com", "e.com"]); - const result = await service.getNextDomains(prev); - expect(result!.domains).toEqual(["d.com", "e.com"]); + fetchWebAddressesSpy.mockResolvedValue(["d.com", "e.com"]); + const result = await service.getNextWebAddresses(prev); + expect(result!.webAddresses).toEqual(["d.com", "e.com"]); expect(result!.checksum).toBe("new"); }); }); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts index 6e1bf07c647..21fe74f1873 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts @@ -20,14 +20,16 @@ import { ScheduledTaskNames, TaskSchedulerService } from "@bitwarden/common/plat import { LogService } from "@bitwarden/logging"; import { GlobalStateProvider, KeyDefinition, PHISHING_DETECTION_DISK } from "@bitwarden/state"; +import { getPhishingResources, PhishingResourceType } from "../phishing-resources"; + export type PhishingData = { - domains: string[]; + webAddresses: string[]; timestamp: number; checksum: string; /** * We store the application version to refetch the entire dataset on a new client release. - * This counteracts daily appends updates not removing inactive or false positive domains. + * This counteracts daily appends updates not removing inactive or false positive web addresses. */ applicationVersion: string; }; @@ -37,34 +39,27 @@ export const PHISHING_DOMAINS_KEY = new KeyDefinition( "phishingDomains", { deserializer: (value: PhishingData) => - value ?? { domains: [], timestamp: 0, checksum: "", applicationVersion: "" }, + value ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }, }, ); -/** Coordinates fetching, caching, and patching of known phishing domains */ +/** Coordinates fetching, caching, and patching of known phishing web addresses */ export class PhishingDataService { - private static readonly RemotePhishingDatabaseUrl = - "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/master/phishing-domains-ACTIVE.txt"; - private static readonly RemotePhishingDatabaseChecksumUrl = - "https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-domains-ACTIVE.txt.md5"; - private static readonly RemotePhishingDatabaseTodayUrl = - "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-domains-NEW-today.txt"; - - private _testDomains = this.getTestDomains(); + private _testWebAddresses = this.getTestWebAddresses(); private _cachedState = this.globalStateProvider.get(PHISHING_DOMAINS_KEY); - private _domains$ = this._cachedState.state$.pipe( + private _webAddresses$ = this._cachedState.state$.pipe( map( (state) => new Set( - (state?.domains?.filter((line) => line.trim().length > 0) ?? []).concat( - this._testDomains, + (state?.webAddresses?.filter((line) => line.trim().length > 0) ?? []).concat( + this._testWebAddresses, "phishing.testcategory.com", // Included for QA to test in prod ), ), ), ); - // How often are new domains added to the remote? + // How often are new web addresses added to the remote? readonly UPDATE_INTERVAL_DURATION = 24 * 60 * 60 * 1000; // 24 hours private _triggerUpdate$ = new Subject(); @@ -75,7 +70,7 @@ export class PhishingDataService { this._cachedState.state$.pipe( first(), // Only take the first value to avoid an infinite loop when updating the cache below switchMap(async (cachedState) => { - const next = await this.getNextDomains(cachedState); + const next = await this.getNextWebAddresses(cachedState); if (next) { await this._cachedState.update(() => next); this.logService.info(`[PhishingDataService] cache updated`); @@ -85,7 +80,7 @@ export class PhishingDataService { count: 3, delay: (err, count) => { this.logService.error( - `[PhishingDataService] Unable to update domains. Attempt ${count}.`, + `[PhishingDataService] Unable to update web addresses. Attempt ${count}.`, err, ); return timer(5 * 60 * 1000); // 5 minutes @@ -97,7 +92,7 @@ export class PhishingDataService { err: unknown /** Eslint actually crashed if you remove this type: https://github.com/cartant/eslint-plugin-rxjs/issues/122 */, ) => { this.logService.error( - "[PhishingDataService] Retries unsuccessful. Unable to update domains.", + "[PhishingDataService] Retries unsuccessful. Unable to update web addresses.", err, ); return EMPTY; @@ -114,6 +109,7 @@ export class PhishingDataService { private globalStateProvider: GlobalStateProvider, private logService: LogService, private platformUtilsService: PlatformUtilsService, + private resourceType: PhishingResourceType = PhishingResourceType.Links, ) { this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.phishingDomainUpdate, () => { this._triggerUpdate$.next(); @@ -125,22 +121,31 @@ export class PhishingDataService { } /** - * Checks if the given URL is a known phishing domain + * Checks if the given URL is a known phishing web address * * @param url The URL to check - * @returns True if the URL is a known phishing domain, false otherwise + * @returns True if the URL is a known phishing web address, false otherwise */ - async isPhishingDomain(url: URL): Promise { - const domains = await firstValueFrom(this._domains$); - const result = domains.has(url.hostname); - if (result) { - return true; + async isPhishingWebAddress(url: URL): Promise { + // Use domain (hostname) matching for domain resources, and link matching for links resources + const entries = await firstValueFrom(this._webAddresses$); + + const resource = getPhishingResources(this.resourceType); + if (resource && resource.match) { + for (const entry of entries) { + if (resource.match(url, entry)) { + return true; + } + } + return false; } - return false; + + // Default/domain behavior: exact hostname match as a fallback + return entries.has(url.hostname); } - async getNextDomains(prev: PhishingData | null): Promise { - prev = prev ?? { domains: [], timestamp: 0, checksum: "", applicationVersion: "" }; + async getNextWebAddresses(prev: PhishingData | null): Promise { + prev = prev ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }; const timestamp = Date.now(); const prevAge = timestamp - prev.timestamp; this.logService.info(`[PhishingDataService] Cache age: ${prevAge}`); @@ -148,7 +153,7 @@ export class PhishingDataService { const applicationVersion = await this.platformUtilsService.getApplicationVersion(); // If checksum matches, return existing data with new timestamp & version - const remoteChecksum = await this.fetchPhishingDomainsChecksum(); + const remoteChecksum = await this.fetchPhishingChecksum(this.resourceType); if (remoteChecksum && prev.checksum === remoteChecksum) { this.logService.info( `[PhishingDataService] Remote checksum matches local checksum, updating timestamp only.`, @@ -157,66 +162,66 @@ export class PhishingDataService { } // Checksum is different, data needs to be updated. - // Approach 1: Fetch only new domains and append + // Approach 1: Fetch only new web addresses and append const isOneDayOldMax = prevAge <= this.UPDATE_INTERVAL_DURATION; if (isOneDayOldMax && applicationVersion === prev.applicationVersion) { - const dailyDomains: string[] = await this.fetchPhishingDomains( - PhishingDataService.RemotePhishingDatabaseTodayUrl, - ); + const webAddressesTodayUrl = getPhishingResources(this.resourceType)!.todayUrl; + const dailyWebAddresses: string[] = + await this.fetchPhishingWebAddresses(webAddressesTodayUrl); this.logService.info( - `[PhishingDataService] ${dailyDomains.length} new phishing domains added`, + `[PhishingDataService] ${dailyWebAddresses.length} new phishing web addresses added`, ); return { - domains: prev.domains.concat(dailyDomains), + webAddresses: prev.webAddresses.concat(dailyWebAddresses), checksum: remoteChecksum, timestamp, applicationVersion, }; } - // Approach 2: Fetch all domains - const domains = await this.fetchPhishingDomains(PhishingDataService.RemotePhishingDatabaseUrl); + // Approach 2: Fetch all web addresses + const remoteUrl = getPhishingResources(this.resourceType)!.remoteUrl; + const remoteWebAddresses = await this.fetchPhishingWebAddresses(remoteUrl); return { - domains, + webAddresses: remoteWebAddresses, timestamp, checksum: remoteChecksum, applicationVersion, }; } - private async fetchPhishingDomainsChecksum() { - const response = await this.apiService.nativeFetch( - new Request(PhishingDataService.RemotePhishingDatabaseChecksumUrl), - ); + private async fetchPhishingChecksum(type: PhishingResourceType = PhishingResourceType.Domains) { + const checksumUrl = getPhishingResources(type)!.checksumUrl; + const response = await this.apiService.nativeFetch(new Request(checksumUrl)); if (!response.ok) { throw new Error(`[PhishingDataService] Failed to fetch checksum: ${response.status}`); } return response.text(); } - private async fetchPhishingDomains(url: string) { + private async fetchPhishingWebAddresses(url: string) { const response = await this.apiService.nativeFetch(new Request(url)); if (!response.ok) { - throw new Error(`[PhishingDataService] Failed to fetch domains: ${response.status}`); + throw new Error(`[PhishingDataService] Failed to fetch web addresses: ${response.status}`); } return response.text().then((text) => text.split("\n")); } - private getTestDomains() { + private getTestWebAddresses() { const flag = devFlagEnabled("testPhishingUrls"); if (!flag) { return []; } - const domains = devFlagValue("testPhishingUrls") as unknown[]; - if (domains && domains instanceof Array) { + const webAddresses = devFlagValue("testPhishingUrls") as unknown[]; + if (webAddresses && webAddresses instanceof Array) { this.logService.debug( - "[PhishingDetectionService] Dev flag enabled for testing phishing detection. Adding test phishing domains:", - domains, + "[PhishingDetectionService] Dev flag enabled for testing phishing detection. Adding test phishing web addresses:", + webAddresses, ); - return domains as string[]; + return webAddresses as string[]; } return []; } diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts index e04d08559ab..d90e872eef8 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts @@ -94,7 +94,7 @@ export class PhishingDetectionService { this._ignoredHostnames.delete(url.hostname); return; } - const isPhishing = await phishingDataService.isPhishingDomain(url); + const isPhishing = await phishingDataService.isPhishingWebAddress(url); if (!isPhishing) { return; } From 5b3e083af3013a1c68eafc819115f7de3f234df6 Mon Sep 17 00:00:00 2001 From: Mick Letofsky Date: Tue, 30 Dec 2025 18:14:54 +0100 Subject: [PATCH 6/7] Review Code Triggered by labeled event (#18151) --- .github/workflows/review-code.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/review-code.yml b/.github/workflows/review-code.yml index 0e0597fccf0..908664209d8 100644 --- a/.github/workflows/review-code.yml +++ b/.github/workflows/review-code.yml @@ -2,7 +2,7 @@ name: Code Review on: pull_request: - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, labeled] permissions: {} From 11b5342df789acf4fdb46aa0e98527148809b769 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 30 Dec 2025 13:03:51 -0600 Subject: [PATCH 7/7] Remove circular invocation / have Account menu use new premium dialog (#17980) --- apps/desktop/src/app/app.component.ts | 4 +++- apps/desktop/src/app/app.module.ts | 10 +++++++++- .../desktop-premium-upgrade-prompt.service.spec.ts | 12 +++++------- .../desktop-premium-upgrade-prompt.service.ts | 6 +++--- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 836328142b5..d75702ee8b8 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -70,6 +70,7 @@ import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; +import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; @@ -198,6 +199,7 @@ export class AppComponent implements OnInit, OnDestroy { private readonly tokenService: TokenService, private desktopAutotypeDefaultSettingPolicy: DesktopAutotypeDefaultSettingPolicy, private readonly lockService: LockService, + private premiumUpgradePromptService: PremiumUpgradePromptService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -305,7 +307,7 @@ export class AppComponent implements OnInit, OnDestroy { await this.openModal(SettingsComponent, this.settingsRef); break; case "openPremium": - this.dialogService.open(PremiumComponent); + await this.premiumUpgradePromptService.promptForPremium(); break; case "showFingerprintPhrase": { const activeUserId = await firstValueFrom( diff --git a/apps/desktop/src/app/app.module.ts b/apps/desktop/src/app/app.module.ts index 31131c6202a..52a52ffd225 100644 --- a/apps/desktop/src/app/app.module.ts +++ b/apps/desktop/src/app/app.module.ts @@ -8,6 +8,7 @@ import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { ColorPasswordCountPipe } from "@bitwarden/angular/pipes/color-password-count.pipe"; import { ColorPasswordPipe } from "@bitwarden/angular/pipes/color-password.pipe"; +import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { CalloutModule, DialogModule } from "@bitwarden/components"; import { AssignCollectionsComponent } from "@bitwarden/vault"; @@ -15,6 +16,7 @@ import { DeleteAccountComponent } from "../auth/delete-account.component"; import { LoginModule } from "../auth/login/login.module"; import { SshAgentService } from "../autofill/services/ssh-agent.service"; import { PremiumComponent } from "../billing/app/accounts/premium.component"; +import { DesktopPremiumUpgradePromptService } from "../services/desktop-premium-upgrade-prompt.service"; import { VaultFilterModule } from "../vault/app/vault/vault-filter/vault-filter.module"; import { VaultV2Component } from "../vault/app/vault/vault-v2.component"; @@ -51,7 +53,13 @@ import { SharedModule } from "./shared/shared.module"; PremiumComponent, SearchComponent, ], - providers: [SshAgentService], + providers: [ + SshAgentService, + { + provide: PremiumUpgradePromptService, + useClass: DesktopPremiumUpgradePromptService, + }, + ], bootstrap: [AppComponent], }) export class AppModule {} diff --git a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts index 1eee4cd54f6..3c36d648167 100644 --- a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts +++ b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts @@ -4,26 +4,24 @@ import { mock, MockProxy } from "jest-mock-extended"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { DialogService } from "@bitwarden/components"; +import { PremiumComponent } from "../billing/app/accounts/premium.component"; + import { DesktopPremiumUpgradePromptService } from "./desktop-premium-upgrade-prompt.service"; describe("DesktopPremiumUpgradePromptService", () => { let service: DesktopPremiumUpgradePromptService; - let messager: MockProxy; let configService: MockProxy; let dialogService: MockProxy; beforeEach(async () => { - messager = mock(); configService = mock(); dialogService = mock(); await TestBed.configureTestingModule({ providers: [ DesktopPremiumUpgradePromptService, - { provide: MessagingService, useValue: messager }, { provide: ConfigService, useValue: configService }, { provide: DialogService, useValue: dialogService }, ], @@ -52,10 +50,10 @@ describe("DesktopPremiumUpgradePromptService", () => { FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog, ); expect(openSpy).toHaveBeenCalledWith(dialogService); - expect(messager.send).not.toHaveBeenCalled(); + expect(dialogService.open).not.toHaveBeenCalled(); }); - it("sends openPremium message when feature flag is disabled", async () => { + it("opens the PremiumComponent when feature flag is disabled", async () => { configService.getFeatureFlag.mockResolvedValue(false); await service.promptForPremium(); @@ -63,7 +61,7 @@ describe("DesktopPremiumUpgradePromptService", () => { expect(configService.getFeatureFlag).toHaveBeenCalledWith( FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog, ); - expect(messager.send).toHaveBeenCalledWith("openPremium"); + expect(dialogService.open).toHaveBeenCalledWith(PremiumComponent); expect(openSpy).not.toHaveBeenCalled(); }); }); diff --git a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts index 5004e5ed547..0161baba801 100644 --- a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts +++ b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts @@ -3,15 +3,15 @@ import { inject } from "@angular/core"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { DialogService } from "@bitwarden/components"; +import { PremiumComponent } from "../billing/app/accounts/premium.component"; + /** * This class handles the premium upgrade process for the desktop. */ export class DesktopPremiumUpgradePromptService implements PremiumUpgradePromptService { - private messagingService = inject(MessagingService); private configService = inject(ConfigService); private dialogService = inject(DialogService); @@ -23,7 +23,7 @@ export class DesktopPremiumUpgradePromptService implements PremiumUpgradePromptS if (showNewDialog) { PremiumUpgradeDialogComponent.open(this.dialogService); } else { - this.messagingService.send("openPremium"); + this.dialogService.open(PremiumComponent); } } }