From 721f253ef9ba5252c2a839a200c7c9384052ab9d Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 15 Dec 2025 16:51:31 +0100 Subject: [PATCH] [PM-28536] Add phishing blocker setting to account security (#17527) * added phishing blocker toggle * design improvements * Fix TypeScript strict mode errors in PhishingDetectionSettingsServiceAbstraction * Camel case messages * Update PhishingDetectionService.initialize parameter ordering * Add comments to PhishingDetectionSettingsServiceAbstraction * Change state from global to user settings * Remove clear on logout phishing-detection-settings * PM-28536 making a change from getActive to getUser because of method being deprecated * Moved phishing detection services to own file * Added new phishing detection availability service to expose complex enable logic * Add test cases for PhishingDetectionAvailabilityService * Remove phishing detection availability in favor of one settings service * Extract phishing detection settings service abstraction to own file * Update phishing detection-settings service to include availability logic. Updated dependencies * Add test cases for phishing detection element. Added missing dependencies in testbed setup * Update services in extension * Switch checkbox to bit-switch component * Remove comment * Remove comment * Fix prettier vs lint spacing * Replace deprecated active user state. Updated test cases * Fix account-security test failing * Update comments * Renamed variable * Removed obsolete message * Remove unused variable * Removed unused import --------- Co-authored-by: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Co-authored-by: Graham Walker Co-authored-by: Tom <144813356+ttalty@users.noreply.github.com> --- apps/browser/src/_locales/en/messages.json | 9 + .../settings/account-security.component.html | 14 ++ .../account-security.component.spec.ts | 145 ++++++++++--- .../settings/account-security.component.ts | 20 ++ .../browser/src/background/main.background.ts | 11 +- .../phishing-detection.service.spec.ts | 20 +- .../services/phishing-detection.service.ts | 28 +-- .../src/popup/services/services.module.ts | 14 ++ ...-detection-settings.service.abstraction.ts | 37 ++++ ...hishing-detection-settings.service.spec.ts | 203 ++++++++++++++++++ .../phishing-detection-settings.service.ts | 116 ++++++++++ 11 files changed, 555 insertions(+), 62 deletions(-) create mode 100644 libs/common/src/dirt/services/abstractions/phishing-detection-settings.service.abstraction.ts create mode 100644 libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts create mode 100644 libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 36d69fb09f5..2ace8ff8b96 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -4801,6 +4801,15 @@ "accountSecurity": { "message": "Account security" }, + "phishingBlocker": { + "message": "Phishing Blocker" + }, + "enablePhishingDetection": { + "message": "Phishing detection" + }, + "enablePhishingDetectionDesc": { + "message": "Display warning before accessing suspected phishing sites" + }, "notifications": { "message": "Notifications" }, diff --git a/apps/browser/src/auth/popup/settings/account-security.component.html b/apps/browser/src/auth/popup/settings/account-security.component.html index b5d725b4a82..bb6b141c6c5 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.html +++ b/apps/browser/src/auth/popup/settings/account-security.component.html @@ -128,6 +128,20 @@ + + +

{{ "phishingBlocker" | i18n }}

+ + + + {{ + "enablePhishingDetection" | i18n + }} + {{ "enablePhishingDetectionDesc" | i18n }} + + + +

{{ "otherOptions" | i18n }}

diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts index d0ab4793301..0f799fe7d4d 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts @@ -3,9 +3,10 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { ActivatedRoute } from "@angular/router"; import { mock } from "jest-mock-extended"; -import { firstValueFrom, of } from "rxjs"; +import { firstValueFrom, of, BehaviorSubject } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; +import { NudgesService } from "@bitwarden/angular/vault"; import { LockService } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -14,12 +15,15 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; +import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { VaultTimeoutSettingsService, VaultTimeoutStringType, VaultTimeoutAction, } from "@bitwarden/common/key-management/vault-timeout"; +import { ProfileResponse } from "@bitwarden/common/models/response/profile.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -27,12 +31,12 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { MessageSender } from "@bitwarden/common/platform/messaging"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; import { StateProvider } from "@bitwarden/common/platform/state"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { DialogService, ToastService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; import { BiometricStateService, BiometricsService, KeyService } from "@bitwarden/key-management"; import { BrowserApi } from "../../../platform/browser/browser-api"; @@ -54,18 +58,27 @@ describe("AccountSecurityComponent", () => { let component: AccountSecurityComponent; let fixture: ComponentFixture; - const mockUserId = Utils.newGuid() as UserId; + const mockUserId = newGuid() as UserId; + const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); - const vaultTimeoutSettingsService = mock(); + const apiService = mock(); + const billingService = mock(); const biometricStateService = mock(); - const policyService = mock(); - const pinServiceAbstraction = mock(); - const keyService = mock(); - const validationService = mock(); - const dialogService = mock(); - const platformUtilsService = mock(); - const lockService = mock(); const configService = mock(); + const dialogService = mock(); + const keyService = mock(); + const lockService = mock(); + const policyService = mock(); + const phishingDetectionSettingsService = mock(); + const pinServiceAbstraction = mock(); + const platformUtilsService = mock(); + const validationService = mock(); + const vaultNudgesService = mock(); + const vaultTimeoutSettingsService = mock(); + + // Mock subjects to control the phishing detection observables + let phishingAvailableSubject: BehaviorSubject; + let phishingEnabledSubject: BehaviorSubject; beforeEach(async () => { await TestBed.configureTestingModule({ @@ -73,29 +86,38 @@ describe("AccountSecurityComponent", () => { { provide: AccountService, useValue: accountService }, { provide: AccountSecurityComponent, useValue: mock() }, { provide: ActivatedRoute, useValue: mock() }, + { provide: ApiService, useValue: apiService }, + { + provide: BillingAccountProfileStateService, + useValue: billingService, + }, { provide: BiometricsService, useValue: mock() }, { provide: BiometricStateService, useValue: biometricStateService }, + { provide: CipherService, useValue: mock() }, + { provide: CollectionService, useValue: mock() }, + { provide: ConfigService, useValue: configService }, { provide: DialogService, useValue: dialogService }, { provide: EnvironmentService, useValue: mock() }, { provide: I18nService, useValue: mock() }, - { provide: MessageSender, useValue: mock() }, { provide: KeyService, useValue: keyService }, + { provide: LockService, useValue: lockService }, + { provide: LogService, useValue: mock() }, + { provide: MessageSender, useValue: mock() }, + { provide: NudgesService, useValue: vaultNudgesService }, + { provide: OrganizationService, useValue: mock() }, { provide: PinServiceAbstraction, useValue: pinServiceAbstraction }, + { + provide: PhishingDetectionSettingsServiceAbstraction, + useValue: phishingDetectionSettingsService, + }, { provide: PlatformUtilsService, useValue: platformUtilsService }, { provide: PolicyService, useValue: policyService }, { provide: PopupRouterCacheService, useValue: mock() }, + { provide: StateProvider, useValue: mock() }, { provide: ToastService, useValue: mock() }, { provide: UserVerificationService, useValue: mock() }, - { provide: VaultTimeoutSettingsService, useValue: vaultTimeoutSettingsService }, - { provide: StateProvider, useValue: mock() }, - { provide: CipherService, useValue: mock() }, - { provide: ApiService, useValue: mock() }, - { provide: LogService, useValue: mock() }, - { provide: OrganizationService, useValue: mock() }, - { provide: CollectionService, useValue: mock() }, { provide: ValidationService, useValue: validationService }, - { provide: LockService, useValue: lockService }, - { provide: ConfigService, useValue: configService }, + { provide: VaultTimeoutSettingsService, useValue: vaultTimeoutSettingsService }, ], }) .overrideComponent(AccountSecurityComponent, { @@ -110,10 +132,13 @@ describe("AccountSecurityComponent", () => { }) .compileComponents(); - fixture = TestBed.createComponent(AccountSecurityComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - + apiService.getProfile.mockResolvedValue( + mock({ + id: mockUserId, + creationDate: new Date().toISOString(), + }), + ); + vaultNudgesService.showNudgeSpotlight$.mockReturnValue(of(false)); vaultTimeoutSettingsService.getVaultTimeoutByUserId$.mockReturnValue( of(VaultTimeoutStringType.OnLocked), ); @@ -123,8 +148,25 @@ describe("AccountSecurityComponent", () => { vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$.mockReturnValue( of(VaultTimeoutAction.Lock), ); + vaultTimeoutSettingsService.availableVaultTimeoutActions$.mockReturnValue(of([])); biometricStateService.promptAutomatically$ = of(false); pinServiceAbstraction.isPinSet.mockResolvedValue(false); + configService.getFeatureFlag$.mockReturnValue(of(false)); + billingService.hasPremiumPersonally$.mockReturnValue(of(true)); + + policyService.policiesByType$.mockReturnValue(of([null])); + + // Mock readonly observables for phishing detection using BehaviorSubjects so + // tests can push different values after component creation. + phishingAvailableSubject = new BehaviorSubject(true); + phishingEnabledSubject = new BehaviorSubject(true); + + (phishingDetectionSettingsService.available$ as any) = phishingAvailableSubject.asObservable(); + (phishingDetectionSettingsService.enabled$ as any) = phishingEnabledSubject.asObservable(); + + fixture = TestBed.createComponent(AccountSecurityComponent); + component = fixture.componentInstance; + fixture.detectChanges(); }); afterEach(() => { @@ -233,6 +275,59 @@ describe("AccountSecurityComponent", () => { expect(pinInputElement).toBeNull(); }); + describe("phishing detection UI and setting", () => { + it("updates phishing detection setting when form value changes", async () => { + policyService.policiesByType$.mockReturnValue(of([null])); + + phishingAvailableSubject.next(true); + phishingEnabledSubject.next(true); + + // Init component + await component.ngOnInit(); + fixture.detectChanges(); + + // Initial form value should match enabled$ observable defaulting to true + expect(component.form.controls.enablePhishingDetection.value).toBe(true); + + // Change the form value to false + component.form.controls.enablePhishingDetection.setValue(false); + fixture.detectChanges(); + // Wait briefly to allow any debounced or async valueChanges handlers to run + // fixture.whenStable() does not work here + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(phishingDetectionSettingsService.setEnabled).toHaveBeenCalledWith(mockUserId, false); + }); + + it("shows phishing detection element when available$ is true", async () => { + policyService.policiesByType$.mockReturnValue(of([null])); + phishingAvailableSubject.next(true); + phishingEnabledSubject.next(true); + + await component.ngOnInit(); + fixture.detectChanges(); + + const phishingDetectionElement = fixture.debugElement.query( + By.css("#phishingDetectionAction"), + ); + expect(phishingDetectionElement).not.toBeNull(); + }); + + it("hides phishing detection element when available$ is false", async () => { + policyService.policiesByType$.mockReturnValue(of([null])); + phishingAvailableSubject.next(false); + phishingEnabledSubject.next(true); + + await component.ngOnInit(); + fixture.detectChanges(); + + const phishingDetectionElement = fixture.debugElement.query( + By.css("#phishingDetectionAction"), + ); + expect(phishingDetectionElement).toBeNull(); + }); + }); + describe("updateBiometric", () => { let browserApiSpy: jest.SpyInstance; diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 4ff29c8853e..7c36754c894 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -32,6 +32,7 @@ import { getFirstPolicy } from "@bitwarden/common/admin-console/services/policy/ import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { @@ -62,6 +63,7 @@ import { SelectModule, TypographyModule, ToastService, + SwitchComponent, } from "@bitwarden/components"; import { KeyService, @@ -110,6 +112,7 @@ import { AwaitDesktopDialogComponent } from "./await-desktop-dialog.component"; SpotlightComponent, TypographyModule, SessionTimeoutInputLegacyComponent, + SwitchComponent, ], }) export class AccountSecurityComponent implements OnInit, OnDestroy { @@ -130,6 +133,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { pinLockWithMasterPassword: false, biometric: false, enableAutoBiometricsPrompt: true, + enablePhishingDetection: true, }); protected showAccountSecurityNudge$: Observable = @@ -141,6 +145,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { ); protected readonly consolidatedSessionTimeoutComponent$: Observable; + protected readonly phishingDetectionAvailable$: Observable; protected refreshTimeoutSettings$ = new BehaviorSubject(undefined); private destroy$ = new Subject(); @@ -167,10 +172,14 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { private vaultNudgesService: NudgesService, private validationService: ValidationService, private logService: LogService, + private phishingDetectionSettingsService: PhishingDetectionSettingsServiceAbstraction, ) { this.consolidatedSessionTimeoutComponent$ = this.configService.getFeatureFlag$( FeatureFlag.ConsolidatedSessionTimeoutComponent, ); + + // Check if user phishing detection available + this.phishingDetectionAvailable$ = this.phishingDetectionSettingsService.available$; } async ngOnInit() { @@ -251,6 +260,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { enableAutoBiometricsPrompt: await firstValueFrom( this.biometricStateService.promptAutomatically$, ), + enablePhishingDetection: await firstValueFrom(this.phishingDetectionSettingsService.enabled$), }; this.form.patchValue(initialValues, { emitEvent: false }); @@ -361,6 +371,16 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { ) .subscribe(); + this.form.controls.enablePhishingDetection.valueChanges + .pipe( + concatMap(async (enabled) => { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + await this.phishingDetectionSettingsService.setEnabled(userId, enabled); + }), + takeUntil(this.destroy$), + ) + .subscribe(); + this.refreshTimeoutSettings$ .pipe( switchMap(() => diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 2540571abb0..7b509380f6d 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -82,7 +82,9 @@ import { import { isUrlInList } from "@bitwarden/common/autofill/utils"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { DefaultBillingAccountProfileStateService } from "@bitwarden/common/billing/services/account/billing-account-profile-state.service"; +import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { HibpApiService } from "@bitwarden/common/dirt/services/hibp-api.service"; +import { PhishingDetectionSettingsService } from "@bitwarden/common/dirt/services/phishing-detection/phishing-detection-settings.service"; import { ClientType } from "@bitwarden/common/enums"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; import { @@ -497,6 +499,7 @@ export default class MainBackground { // DIRT private phishingDataService: PhishingDataService; + private phishingDetectionSettingsService: PhishingDetectionSettingsServiceAbstraction; constructor() { const logoutCallback = async (logoutReason: LogoutReason, userId?: UserId) => @@ -1475,12 +1478,18 @@ export default class MainBackground { this.platformUtilsService, ); - PhishingDetectionService.initialize( + this.phishingDetectionSettingsService = new PhishingDetectionSettingsService( this.accountService, this.billingAccountProfileStateService, this.configService, + this.organizationService, + this.stateProvider, + ); + + PhishingDetectionService.initialize( this.logService, this.phishingDataService, + this.phishingDetectionSettingsService, messageListener, ); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts index e33b4b1b4f1..ceb18bd1573 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts @@ -1,9 +1,7 @@ import { mock, MockProxy } from "jest-mock-extended"; import { Observable, of } from "rxjs"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessageListener } from "@bitwarden/messaging"; @@ -11,17 +9,12 @@ import { PhishingDataService } from "./phishing-data.service"; import { PhishingDetectionService } from "./phishing-detection.service"; describe("PhishingDetectionService", () => { - let accountService: AccountService; - let billingAccountProfileStateService: BillingAccountProfileStateService; - let configService: ConfigService; let logService: LogService; let phishingDataService: MockProxy; let messageListener: MockProxy; + let phishingDetectionSettingsService: MockProxy; beforeEach(() => { - accountService = { getAccount$: jest.fn(() => of(null)) } as any; - billingAccountProfileStateService = {} as any; - configService = { getFeatureFlag$: jest.fn(() => of(false)) } as any; logService = { info: jest.fn(), debug: jest.fn(), warning: jest.fn(), error: jest.fn() } as any; phishingDataService = mock(); messageListener = mock({ @@ -29,16 +22,17 @@ describe("PhishingDetectionService", () => { return new Observable(); }, }); + phishingDetectionSettingsService = mock({ + on$: of(true), + }); }); it("should initialize without errors", () => { expect(() => { PhishingDetectionService.initialize( - accountService, - billingAccountProfileStateService, - configService, logService, phishingDataService, + phishingDetectionSettingsService, messageListener, ); }).not.toThrow(); @@ -61,6 +55,7 @@ describe("PhishingDetectionService", () => { // logService, // phishingDataService, // messageListener, + // phishingDetectionSettingsService, // ); // }); @@ -81,6 +76,7 @@ describe("PhishingDetectionService", () => { // logService, // phishingDataService, // messageListener, + // phishingDetectionSettingsService, // ); // }); }); 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 4917e740be8..e04d08559ab 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 @@ -1,21 +1,16 @@ import { - combineLatest, concatMap, distinctUntilChanged, EMPTY, filter, map, merge, - of, Subject, switchMap, tap, } from "rxjs"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { CommandDefinition, MessageListener } from "@bitwarden/messaging"; @@ -50,11 +45,9 @@ export class PhishingDetectionService { private static _didInit = false; static initialize( - accountService: AccountService, - billingAccountProfileStateService: BillingAccountProfileStateService, - configService: ConfigService, logService: LogService, phishingDataService: PhishingDataService, + phishingDetectionSettingsService: PhishingDetectionSettingsServiceAbstraction, messageListener: MessageListener, ) { if (this._didInit) { @@ -118,22 +111,9 @@ export class PhishingDetectionService { .messages$(PHISHING_DETECTION_CANCEL_COMMAND) .pipe(switchMap((message) => BrowserApi.closeTab(message.tabId))); - const activeAccountHasAccess$ = combineLatest([ - accountService.activeAccount$, - configService.getFeatureFlag$(FeatureFlag.PhishingDetection), - ]).pipe( - switchMap(([account, featureEnabled]) => { - if (!account) { - logService.debug("[PhishingDetectionService] No active account."); - return of(false); - } - return billingAccountProfileStateService - .hasPremiumFromAnySource$(account.id) - .pipe(map((hasPremium) => hasPremium && featureEnabled)); - }), - ); + const phishingDetectionActive$ = phishingDetectionSettingsService.on$; - const initSub = activeAccountHasAccess$ + const initSub = phishingDetectionActive$ .pipe( distinctUntilChanged(), switchMap((activeUserHasAccess) => { diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index bb89eff1147..39c53b7da56 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -41,6 +41,7 @@ import { import { ExtensionNewDeviceVerificationComponentService } from "@bitwarden/browser/auth/services/new-device-verification/extension-new-device-verification-component.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService, @@ -67,6 +68,8 @@ import { UserNotificationSettingsServiceAbstraction, } from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; +import { PhishingDetectionSettingsService } from "@bitwarden/common/dirt/services/phishing-detection/phishing-detection-settings.service"; import { ClientType } from "@bitwarden/common/enums"; import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; @@ -512,6 +515,17 @@ const safeProviders: SafeProvider[] = [ useClass: UserNotificationSettingsService, deps: [StateProvider], }), + safeProvider({ + provide: PhishingDetectionSettingsServiceAbstraction, + useClass: PhishingDetectionSettingsService, + deps: [ + AccountService, + BillingAccountProfileStateService, + ConfigService, + OrganizationService, + StateProvider, + ], + }), safeProvider({ provide: MessageListener, useFactory: (subject: Subject>>, ngZone: NgZone) => diff --git a/libs/common/src/dirt/services/abstractions/phishing-detection-settings.service.abstraction.ts b/libs/common/src/dirt/services/abstractions/phishing-detection-settings.service.abstraction.ts new file mode 100644 index 00000000000..6c915c2dcbe --- /dev/null +++ b/libs/common/src/dirt/services/abstractions/phishing-detection-settings.service.abstraction.ts @@ -0,0 +1,37 @@ +import { Observable } from "rxjs"; + +import { UserId } from "@bitwarden/user-core"; + +/** + * Abstraction for phishing detection settings + */ +export abstract class PhishingDetectionSettingsServiceAbstraction { + /** + * An observable for whether phishing detection is available for the active user account. + * + * Access is granted only when the PhishingDetection feature flag is enabled and + * at least one of the following is true for the active account: + * - the user has a personal premium subscription + * - the user is a member of a Family org (ProductTierType.Families) + * - the user is a member of an Enterprise org with `usePhishingBlocker` enabled + * + * Note: Non-specified organization types (e.g., Team orgs) do not grant access. + */ + abstract readonly available$: Observable; + /** + * An observable for whether phishing detection is on for the active user account + * + * This is true when {@link available$} is true and when {@link enabled$} is true + */ + abstract readonly on$: Observable; + /** + * An observable for whether phishing detection is enabled + */ + abstract readonly enabled$: Observable; + /** + * Sets whether phishing detection is enabled + * + * @param enabled True to enable, false to disable + */ + abstract setEnabled: (userId: UserId, enabled: boolean) => Promise; +} diff --git a/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts new file mode 100644 index 00000000000..23e311d9445 --- /dev/null +++ b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts @@ -0,0 +1,203 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, firstValueFrom, Subject } from "rxjs"; + +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; + +import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; +import { UserId } from "../../../types/guid"; + +import { PhishingDetectionSettingsService } from "./phishing-detection-settings.service"; + +describe("PhishingDetectionSettingsService", () => { + // Mock services + let mockAccountService: MockProxy; + let mockBillingService: MockProxy; + let mockConfigService: MockProxy; + let mockOrganizationService: MockProxy; + + // RxJS Subjects we control in the tests + let activeAccountSubject: BehaviorSubject; + let featureFlagSubject: BehaviorSubject; + let premiumStatusSubject: BehaviorSubject; + let organizationsSubject: BehaviorSubject; + + let service: PhishingDetectionSettingsService; + let stateProvider: FakeStateProvider; + + // Constant mock data + const familyOrg = mock({ + canAccess: true, + isMember: true, + usersGetPremium: true, + productTierType: ProductTierType.Families, + usePhishingBlocker: true, + }); + const teamOrg = mock({ + canAccess: true, + isMember: true, + usersGetPremium: true, + productTierType: ProductTierType.Teams, + usePhishingBlocker: true, + }); + const enterpriseOrg = mock({ + canAccess: true, + isMember: true, + usersGetPremium: true, + productTierType: ProductTierType.Enterprise, + usePhishingBlocker: true, + }); + + const mockUserId = "mock-user-id" as UserId; + const account = mock({ id: mockUserId }); + const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); + + beforeEach(() => { + // Initialize subjects + activeAccountSubject = new BehaviorSubject(null); + featureFlagSubject = new BehaviorSubject(false); + premiumStatusSubject = new BehaviorSubject(false); + organizationsSubject = new BehaviorSubject([]); + + // Default implementations for required functions + mockAccountService = mock(); + mockAccountService.activeAccount$ = activeAccountSubject.asObservable(); + + mockBillingService = mock(); + mockBillingService.hasPremiumPersonally$.mockReturnValue(premiumStatusSubject.asObservable()); + + mockConfigService = mock(); + mockConfigService.getFeatureFlag$.mockReturnValue(featureFlagSubject.asObservable()); + + mockOrganizationService = mock(); + mockOrganizationService.organizations$.mockReturnValue(organizationsSubject.asObservable()); + + stateProvider = new FakeStateProvider(accountService); + service = new PhishingDetectionSettingsService( + mockAccountService, + mockBillingService, + mockConfigService, + mockOrganizationService, + stateProvider, + ); + }); + + // Helper to easily get the result of the observable we are testing + const getAccess = () => firstValueFrom(service.available$); + + describe("enabled$", () => { + it("should default to true if an account is logged in", async () => { + activeAccountSubject.next(account); + const result = await firstValueFrom(service.enabled$); + expect(result).toBe(true); + }); + + it("should return the stored value", async () => { + activeAccountSubject.next(account); + + await service.setEnabled(mockUserId, false); + const resultDisabled = await firstValueFrom(service.enabled$); + expect(resultDisabled).toBe(false); + + await service.setEnabled(mockUserId, true); + const resultEnabled = await firstValueFrom(service.enabled$); + expect(resultEnabled).toBe(true); + }); + }); + + describe("setEnabled", () => { + it("should update the stored value", async () => { + activeAccountSubject.next(account); + await service.setEnabled(mockUserId, false); + let result = await firstValueFrom(service.enabled$); + expect(result).toBe(false); + + await service.setEnabled(mockUserId, true); + result = await firstValueFrom(service.enabled$); + expect(result).toBe(true); + }); + }); + + it("returns false immediately when the feature flag is disabled, regardless of other conditions", async () => { + activeAccountSubject.next(account); + premiumStatusSubject.next(true); + organizationsSubject.next([familyOrg]); + + featureFlagSubject.next(false); + + await expect(getAccess()).resolves.toBe(false); + }); + + it("returns false if there is no active account present yet", async () => { + activeAccountSubject.next(null); // No active account + featureFlagSubject.next(true); // Flag is on + + await expect(getAccess()).resolves.toBe(false); + }); + + it("returns true when feature flag is enabled and user has premium personally", async () => { + activeAccountSubject.next(account); + featureFlagSubject.next(true); + organizationsSubject.next([]); + premiumStatusSubject.next(true); + + await expect(getAccess()).resolves.toBe(true); + }); + + it("returns true when feature flag is enabled and user is in a Family Organization", async () => { + activeAccountSubject.next(account); + featureFlagSubject.next(true); + premiumStatusSubject.next(false); // User has no personal premium + + organizationsSubject.next([familyOrg]); + + await expect(getAccess()).resolves.toBe(true); + }); + + it("returns true when feature flag is enabled and user is in an Enterprise org with phishing blocker enabled", async () => { + activeAccountSubject.next(account); + featureFlagSubject.next(true); + premiumStatusSubject.next(false); + organizationsSubject.next([enterpriseOrg]); + + await expect(getAccess()).resolves.toBe(true); + }); + + it("returns false when user has no access through personal premium or organizations", async () => { + activeAccountSubject.next(account); + featureFlagSubject.next(true); + premiumStatusSubject.next(false); + organizationsSubject.next([teamOrg]); // Team org does not give access + + await expect(getAccess()).resolves.toBe(false); + }); + + it("shares/caches the available$ result between multiple subscribers", async () => { + // Use a plain Subject for this test so we control when the premium observable emits + // and avoid the BehaviorSubject's initial emission which can race with subscriptions. + // Provide the Subject directly as the mock return value for the billing service + const oneTimePremium = new Subject(); + mockBillingService.hasPremiumPersonally$.mockReturnValueOnce(oneTimePremium.asObservable()); + + activeAccountSubject.next(account); + featureFlagSubject.next(true); + organizationsSubject.next([]); + + const p1 = firstValueFrom(service.available$); + const p2 = firstValueFrom(service.available$); + + // Trigger the pipeline + oneTimePremium.next(true); + + const [first, second] = await Promise.all([p1, p2]); + + expect(first).toBe(true); + expect(second).toBe(true); + // The billing function should have been called at most once due to caching + expect(mockBillingService.hasPremiumPersonally$).toHaveBeenCalledTimes(1); + }); +}); diff --git a/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts new file mode 100644 index 00000000000..36d50f60de7 --- /dev/null +++ b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts @@ -0,0 +1,116 @@ +import { combineLatest, Observable, of, switchMap } from "rxjs"; +import { catchError, distinctUntilChanged, map, shareReplay } from "rxjs/operators"; + +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { UserId } from "@bitwarden/user-core"; + +import { PHISHING_DETECTION_DISK, StateProvider, UserKeyDefinition } from "../../../platform/state"; +import { PhishingDetectionSettingsServiceAbstraction } from "../abstractions/phishing-detection-settings.service.abstraction"; + +const ENABLE_PHISHING_DETECTION = new UserKeyDefinition( + PHISHING_DETECTION_DISK, + "enablePhishingDetection", + { + deserializer: (value: boolean) => value ?? true, // Default: enabled + clearOn: [], + }, +); + +export class PhishingDetectionSettingsService implements PhishingDetectionSettingsServiceAbstraction { + readonly available$: Observable; + readonly enabled$: Observable; + readonly on$: Observable; + + constructor( + private accountService: AccountService, + private billingService: BillingAccountProfileStateService, + private configService: ConfigService, + private organizationService: OrganizationService, + private stateProvider: StateProvider, + ) { + this.available$ = this.buildAvailablePipeline$().pipe( + distinctUntilChanged(), + shareReplay({ bufferSize: 1, refCount: true }), + ); + this.enabled$ = this.buildEnabledPipeline$().pipe( + distinctUntilChanged(), + shareReplay({ bufferSize: 1, refCount: true }), + ); + + this.on$ = combineLatest([this.available$, this.enabled$]).pipe( + map(([available, enabled]) => available && enabled), + distinctUntilChanged(), + shareReplay({ bufferSize: 1, refCount: true }), + ); + } + + async setEnabled(userId: UserId, enabled: boolean): Promise { + await this.stateProvider.getUser(userId, ENABLE_PHISHING_DETECTION).update(() => enabled); + } + + /** + * Builds the observable pipeline to determine if phishing detection is available to the user + * + * @returns An observable pipeline that determines if phishing detection is available + */ + private buildAvailablePipeline$(): Observable { + return combineLatest([ + this.accountService.activeAccount$, + this.configService.getFeatureFlag$(FeatureFlag.PhishingDetection), + ]).pipe( + switchMap(([account, featureEnabled]) => { + if (!account || !featureEnabled) { + return of(false); + } + return combineLatest([ + this.billingService.hasPremiumPersonally$(account.id).pipe(catchError(() => of(false))), + this.organizationService.organizations$(account.id).pipe(catchError(() => of([]))), + ]).pipe( + map(([hasPremium, organizations]) => hasPremium || this.orgGrantsAccess(organizations)), + catchError(() => of(false)), + ); + }), + ); + } + + /** + * Builds the observable pipeline to determine if phishing detection is enabled by the user + * + * @returns True if phishing detection is enabled for the active user + */ + private buildEnabledPipeline$(): Observable { + return this.accountService.activeAccount$.pipe( + switchMap((account) => { + if (!account) { + return of(false); + } + return this.stateProvider.getUserState$(ENABLE_PHISHING_DETECTION, account.id); + }), + map((enabled) => enabled ?? true), + ); + } + + /** + * Determines if any of the user's organizations grant access to phishing detection + * + * @param organizations The organizations the user is a member of + * @returns True if any organization grants access to phishing detection + */ + private orgGrantsAccess(organizations: Organization[]): boolean { + return organizations.some((org) => { + if (!org.canAccess || !org.isMember || !org.usersGetPremium) { + return false; + } + return ( + org.productTierType === ProductTierType.Families || + (org.productTierType === ProductTierType.Enterprise && org.usePhishingBlocker) + ); + }); + } +}