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,