From ea4666e3c1f70d3bcada332ab301627c1a58ac2c Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Fri, 19 Dec 2025 11:58:14 -0600 Subject: [PATCH] [PM-25884] Move Phishing Detection Safari check to PhishingDetectionSettingsService (#18042) * Move safari check to phishing detection settings to expose to all places using phishing detection * Remove duplicate comment --- apps/browser/src/background/main.background.ts | 1 + .../services/phishing-detection.service.ts | 13 +------------ apps/browser/src/popup/services/services.module.ts | 1 + .../phishing-detection-settings.service.spec.ts | 5 +++++ .../phishing-detection-settings.service.ts | 7 +++++++ 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index c224dcf581e..2d1da8510c1 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1490,6 +1490,7 @@ export default class MainBackground { this.billingAccountProfileStateService, this.configService, this.organizationService, + this.platformUtilsService, this.stateProvider, ); 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 501dfbf7a50..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 @@ -5,7 +5,6 @@ import { filter, map, merge, - of, Subject, switchMap, tap, @@ -112,17 +111,7 @@ export class PhishingDetectionService { .messages$(PHISHING_DETECTION_CANCEL_COMMAND) .pipe(switchMap((message) => BrowserApi.closeTab(message.tabId))); - // Phishing detection is unavailable on Safari due to platform limitations - if (BrowserApi.isSafariApi) { - logService.debug( - "[PhishingDetectionService] Disabling phishing detection service for Safari.", - ); - } - - // Watching for settings changes to enable/disable phishing detection - const phishingDetectionActive$ = BrowserApi.isSafariApi - ? of(false) - : phishingDetectionSettingsService.on$; + const phishingDetectionActive$ = phishingDetectionSettingsService.on$; const initSub = phishingDetectionActive$ .pipe( diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 39c53b7da56..739166ff6f8 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -523,6 +523,7 @@ const safeProviders: SafeProvider[] = [ BillingAccountProfileStateService, ConfigService, OrganizationService, + PlatformUtilsService, StateProvider, ], }), 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 index 23e311d9445..e6363b490cb 100644 --- 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 @@ -7,6 +7,7 @@ import { Account, AccountService } from "@bitwarden/common/auth/abstractions/acc 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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { UserId } from "../../../types/guid"; @@ -19,6 +20,7 @@ describe("PhishingDetectionSettingsService", () => { let mockBillingService: MockProxy; let mockConfigService: MockProxy; let mockOrganizationService: MockProxy; + let mockPlatformService: MockProxy; // RxJS Subjects we control in the tests let activeAccountSubject: BehaviorSubject; @@ -76,12 +78,15 @@ describe("PhishingDetectionSettingsService", () => { mockOrganizationService = mock(); mockOrganizationService.organizations$.mockReturnValue(organizationsSubject.asObservable()); + mockPlatformService = mock(); + stateProvider = new FakeStateProvider(accountService); service = new PhishingDetectionSettingsService( mockAccountService, mockBillingService, mockConfigService, mockOrganizationService, + mockPlatformService, stateProvider, ); }); 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 index 36d50f60de7..e30592b2f68 100644 --- 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 @@ -8,6 +8,7 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs 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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { UserId } from "@bitwarden/user-core"; import { PHISHING_DETECTION_DISK, StateProvider, UserKeyDefinition } from "../../../platform/state"; @@ -32,6 +33,7 @@ export class PhishingDetectionSettingsService implements PhishingDetectionSettin private billingService: BillingAccountProfileStateService, private configService: ConfigService, private organizationService: OrganizationService, + private platformService: PlatformUtilsService, private stateProvider: StateProvider, ) { this.available$ = this.buildAvailablePipeline$().pipe( @@ -60,6 +62,11 @@ export class PhishingDetectionSettingsService implements PhishingDetectionSettin * @returns An observable pipeline that determines if phishing detection is available */ private buildAvailablePipeline$(): Observable { + // Phishing detection is unavailable on Safari due to platform limitations. + if (this.platformService.isSafari()) { + return of(false); + } + return combineLatest([ this.accountService.activeAccount$, this.configService.getFeatureFlag$(FeatureFlag.PhishingDetection),