From 06d77ffefdcd6abea4d0611e009940705801f390 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Thu, 5 Sep 2024 14:08:29 +0100 Subject: [PATCH 01/28] Add tests --- DuckDuckGo.xcodeproj/project.pbxproj | 20 ++ .../PhishingDetectionIntegrationTests.swift | 176 ++++++++++++++++++ .../Mocks/PhishingDetectionMocks.swift | 47 +++++ .../PhishingDetectionTests.swift | 89 +++++++++ 4 files changed, 332 insertions(+) create mode 100644 IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift create mode 100644 UnitTests/PhishingDetection/PhishingDetectionTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 803b261ffc..457cb46cc6 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2697,6 +2697,10 @@ CD34F0C12C886482006826BE /* PhishingDetectionMocks.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD34F0BF2C886482006826BE /* PhishingDetectionMocks.swift */; }; CD34F0C22C886482006826BE /* PhishingDetectionMocks.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD34F0BF2C886482006826BE /* PhishingDetectionMocks.swift */; }; CD34F0C42C8869FF006826BE /* PhishingDetection in Frameworks */ = {isa = PBXBuildFile; productRef = CD34F0C32C8869FF006826BE /* PhishingDetection */; }; + CD89DD612C89E08D0080F9AF /* PhishingDetectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD89DD5D2C89E08D0080F9AF /* PhishingDetectionTests.swift */; }; + CD89DD622C89E08D0080F9AF /* PhishingDetectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD89DD5D2C89E08D0080F9AF /* PhishingDetectionTests.swift */; }; + CD89DD652C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD89DD632C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift */; }; + CD89DD662C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD89DD632C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift */; }; D64A5FF82AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */; }; D64A5FF92AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */; }; D6BC8AC62C5A95AA0025375B /* DuckPlayer in Frameworks */ = {isa = PBXBuildFile; productRef = D6BC8AC52C5A95AA0025375B /* DuckPlayer */; }; @@ -4399,6 +4403,8 @@ CD33012B2C89B588009AA127 /* ErrorPageHTMLFactory.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ErrorPageHTMLFactory.swift; sourceTree = ""; }; CD33012F2C89B602009AA127 /* ErrorPageHTMLFactoryTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ErrorPageHTMLFactoryTests.swift; sourceTree = ""; }; CD34F0BF2C886482006826BE /* PhishingDetectionMocks.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PhishingDetectionMocks.swift; sourceTree = ""; }; + CD89DD5D2C89E08D0080F9AF /* PhishingDetectionTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PhishingDetectionTests.swift; sourceTree = ""; }; + CD89DD632C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PhishingDetectionIntegrationTests.swift; sourceTree = ""; }; CDE248A32C821FFE00F9399D /* hashPrefixes.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = hashPrefixes.json; sourceTree = ""; }; CDE248A42C821FFE00F9399D /* PhishingDetection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PhishingDetection.swift; sourceTree = ""; }; CDE248A52C821FFE00F9399D /* PhishingDetectionPreferences.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PhishingDetectionPreferences.swift; sourceTree = ""; }; @@ -5463,6 +5469,7 @@ B62A233E29C41D2D00D22475 /* History */, B603973229BEF84900902A34 /* HTTPSUpgrade */, B62A233A29C322A000D22475 /* NavigationProtection */, + CD89DD642C89E0BB0080F9AF /* PhishingDetection */, B603973629BF0E9400902A34 /* PrivacyDashboard */, B644B43C29D56811003FA9AB /* Tab */, 4B1AD91625FC46FB00261379 /* CoreDataEncryptionTests.swift */, @@ -8761,6 +8768,7 @@ isa = PBXGroup; children = ( CD34F0C02C886482006826BE /* Mocks */, + CD89DD5D2C89E08D0080F9AF /* PhishingDetectionTests.swift */, ); path = PhishingDetection; sourceTree = ""; @@ -8773,6 +8781,14 @@ path = Mocks; sourceTree = ""; }; + CD89DD642C89E0BB0080F9AF /* PhishingDetection */ = { + isa = PBXGroup; + children = ( + CD89DD632C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift */, + ); + path = PhishingDetection; + sourceTree = ""; + }; CDE248A22C821FD500F9399D /* PhishingDetection */ = { isa = PBXGroup; children = ( @@ -11312,6 +11328,7 @@ 3706FE75293F661700E42796 /* WebsiteBreakageReportTests.swift in Sources */, 56D145EF29E6DAD900E3488A /* DataImportProviderTests.swift in Sources */, 569277C529DEE09D00B633EF /* ContinueSetUpModelTests.swift in Sources */, + CD89DD622C89E08D0080F9AF /* PhishingDetectionTests.swift in Sources */, 3706FE76293F661700E42796 /* MockSecureVault.swift in Sources */, F1AFDBD92C23221700710F2C /* SubscriptionAppStoreRestorerTests.swift in Sources */, C1E961F32B87B273001760E1 /* MockAutofillActionExecutor.swift in Sources */, @@ -11382,6 +11399,7 @@ 3706FEA5293F662100E42796 /* CoreDataEncryptionTesting.xcdatamodeld in Sources */, B603973D29BF1D7D00902A34 /* AutoconsentIntegrationTests.swift in Sources */, B60C6F8729B1CAB2007BFAA8 /* TestRunHelper.swift in Sources */, + CD89DD662C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift in Sources */, B64CE01F2B8622D700126CA5 /* AddressBarTests.swift in Sources */, B603972D29BEDF2100902A34 /* ExpectedNavigationExtension.swift in Sources */, 3706FEA6293F662100E42796 /* EncryptionKeyStoreTests.swift in Sources */, @@ -11426,6 +11444,7 @@ B644B43D29D56829003FA9AB /* SearchNonexistentDomainTests.swift in Sources */, B603973C29BF1D7D00902A34 /* AutoconsentIntegrationTests.swift in Sources */, B60C6F8629B1CAB0007BFAA8 /* TestRunHelper.swift in Sources */, + CD89DD652C89E0BB0080F9AF /* PhishingDetectionIntegrationTests.swift in Sources */, B64CE01E2B8622D700126CA5 /* AddressBarTests.swift in Sources */, B603972C29BEDF2100902A34 /* ExpectedNavigationExtension.swift in Sources */, 4B1AD8D525FC38DD00261379 /* EncryptionKeyStoreTests.swift in Sources */, @@ -12715,6 +12734,7 @@ 857E5AFA2A7961FF00FC0FB4 /* PixelExperimentTests.swift in Sources */, AAC9C01524CAFBCE00AD1325 /* TabTests.swift in Sources */, B69B504C2726CA2900758A2B /* MockVariantManager.swift in Sources */, + CD89DD612C89E08D0080F9AF /* PhishingDetectionTests.swift in Sources */, 310E79BF294A19A8007C49E8 /* FireproofingReferenceTests.swift in Sources */, B6BBF1722744CE36004F850E /* FireproofDomainsStoreMock.swift in Sources */, 4BA1A6D9258C0CB300F6F690 /* DataEncryptionTests.swift in Sources */, diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift new file mode 100644 index 0000000000..9af201a33f --- /dev/null +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -0,0 +1,176 @@ +// +// PhishingDetectionIntegrationTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Combine +import Common +import XCTest + +@testable import DuckDuckGo_Privacy_Browser + +@available(macOS 12.0, *) +class PhishingDetectionIntegrationTests: XCTestCase { + + var window: NSWindow! + var tabViewModel: TabViewModel { + (window.contentViewController as! MainViewController).browserTabViewController.tabViewModel! + } + + @MainActor + override func setUp() { + // disable GPC redirects + WebTrackingProtectionPreferences.shared.isGPCEnabled = false + PhishingDetectionPreferences.shared.isEnabled = true + + window = WindowsManager.openNewWindow(with: .none)! + } + + @MainActor + override func tearDown() async throws { + window.close() + window = nil + + WebTrackingProtectionPreferences.shared.isGPCEnabled = true + } + + // MARK: - Tests + @MainActor + func testWhenPhishingNotDetected_tabIsNotMarkedPhishing() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load the test page + let url = URL(string: "http://privacy-test-pages.site/")! + _=await tab.setUrl(url, source: .link)?.result + + let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? true + XCTAssertFalse(isPhishingErrorPage) + } + + @MainActor + func testWhenPhishingDetected_tabIsMarkedPhishing() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load fake phishing test page + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + _=await tab.setUrl(url, source: .link)?.result + + let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + } + + @MainActor + func testWhenPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load fake phishing test page + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + _=await tab.setUrl(url, source: .link)?.result + var isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + XCTAssertTrue(isPhishingErrorPage) + try await Task.sleep(nanoseconds: 1_000_000_000) + let url2 = URL(string: "http://broken.third-party.site/")! + _=await tab.setUrl(url2, source: .link)?.result + try await Task.sleep(nanoseconds: 1_000_000_000) + let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? true + isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? true + XCTAssertFalse(privacyInfoPhishing) + XCTAssertFalse(isPhishingErrorPage) + } + + @MainActor + func testWhenPhishingWarningClickedThrough_tabIsMarkedAsBypassed() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load fake phishing test page - errorPageType = Phishing + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + _=await tab.setUrl(url, source: .link)?.result + var isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + var privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + while tab.isLoading { + try await Task.sleep(nanoseconds: 1_000_000_000) + } + // now visit site + let showAdvancedScript: String = "document.getElementsByClassName('Warning_advanced')[0].click()" + try? await tab.webView.evaluateJavaScript(showAdvancedScript) as Void? + let clickThroughScript: String = "document.getElementsByClassName('AdvancedInfo_visitSite')[0].click()" + try? await tab.webView.evaluateJavaScript(clickThroughScript) as Void? + try await Task.sleep(nanoseconds: 1_000_000_000) + isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + } + + @MainActor + func testWhenPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load fake phishing test page + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + _=await tab.setUrl(url, source: .link)?.result + var isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + var privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + try await Task.sleep(nanoseconds: 1_000_000_000) + // we have special exceptions for DDG URLs, which previously caused a bug where phishing sites that quickly navigated to DDG caused a broken phishingState + let url2 = URL(string: "http://duckduckgo.com/")! + _=await tab.setUrl(url2, source: .link)?.result + try await Task.sleep(nanoseconds: 1_000_000_000) + privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + } + + @MainActor + func testWhenPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load fake phishing test page + let url = URL(string: "http://bad.third-party.site/security/badware/phishing-iframe-loader.html")! + _=await tab.setUrl(url, source: .link)?.result + let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + } + + @MainActor + func testWhenPhishingDetectedViaRedirectChain_tabIsMarkedPhishing() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + // load fake phishing test page with redirector + let url = URL(string: "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html")! + _=await tab.setUrl(url, source: .link)?.result + let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false + let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false + XCTAssertTrue(privacyInfoPhishing) + XCTAssertTrue(isPhishingErrorPage) + } +} diff --git a/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift b/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift index a3682944bd..9cbdebf227 100644 --- a/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift +++ b/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift @@ -22,6 +22,53 @@ import Combine import PhishingDetection @testable import DuckDuckGo_Privacy_Browser +class MockDataProvider: PhishingDetectionDataProviding { + var embeddedFilterSet: Set = [] + var embeddedHashPrefixes: Set = [] + var embeddedRevision: Int = 0 + var didLoadFilterSet: Bool = false + var didLoadHashPrefixes: Bool = false + + func loadEmbeddedFilterSet() -> Set { + didLoadFilterSet = true + return embeddedFilterSet + } + + func loadEmbeddedHashPrefixes() -> Set { + didLoadHashPrefixes = true + return embeddedHashPrefixes + } +} + +class MockFileStorageManager: FileStorageManager { + private var storage: [String: Data] = [:] + var didWriteToDisk: Bool = false + var didReadFromDisk: Bool = false + + func write(data: Data, to filename: String) { + didWriteToDisk = true + storage[filename] = data + } + + func read(from filename: String) -> Data? { + didReadFromDisk = true + return storage[filename] + } +} + +final class MockPhishingDataActivitites: PhishingDetectionDataActivityHandling { + var started: Bool = false + var stopped: Bool = false + + func start() { + started = true + } + + func stop() { + stopped = true + } +} + final class MockPhishingDetection: PhishingDetecting { func isMalicious(url: URL) async -> Bool { return url.absoluteString.contains("malicious") diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift new file mode 100644 index 0000000000..27292ae73d --- /dev/null +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -0,0 +1,89 @@ +// +// PhishingDetectionTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import XCTest +import Combine +import PhishingDetection +@testable import DuckDuckGo_Privacy_Browser + +final class PhishingDetectionTests: XCTestCase { + var phishingDetection: PhishingDetection! + var dataStore: PhishingDetectionDataStore! + var mockDataActivities: MockPhishingDataActivitites! + var mockDetector: MockPhishingDetection! + var mockFileStorageManager: MockFileStorageManager! + var mockDataProvider: MockDataProvider! + + override func setUp() { + mockDataActivities = MockPhishingDataActivitites() + mockDetector = MockPhishingDetection() + mockFileStorageManager = MockFileStorageManager() + mockDataProvider = MockDataProvider() + dataStore = PhishingDetectionDataStore(dataProvider: mockDataProvider, fileStorageManager: mockFileStorageManager) + phishingDetection = PhishingDetection(dataActivities: mockDataActivities, dataStore: dataStore, detector: mockDetector) + super.setUp() + } + + override func tearDown() { + phishingDetection = nil + mockDataProvider = nil + mockFileStorageManager = nil + mockDataActivities = nil + mockDetector = nil + super.tearDown() + } + + func testDidLoadAndStartDataActivities() async { + XCTAssertTrue(mockDataActivities.started) + } + + func testDisableFeature() async { + PhishingDetectionPreferences.shared.isEnabled = false + let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) + XCTAssertFalse(isMalicious) + XCTAssertTrue(mockDataActivities.stopped) + } + + func testDidNotLoadAndStartDataActivities_IfFeatureDisabled() async { + PhishingDetectionPreferences.shared.isEnabled = false + mockDataActivities = MockPhishingDataActivitites() + mockFileStorageManager = MockFileStorageManager() + mockDataProvider = MockDataProvider() + dataStore = PhishingDetectionDataStore(dataProvider: mockDataProvider, fileStorageManager: mockFileStorageManager) + phishingDetection = PhishingDetection(dataActivities: mockDataActivities, dataStore: dataStore, detector: mockDetector) + XCTAssertFalse(mockDataProvider.didLoadHashPrefixes) + XCTAssertFalse(mockDataProvider.didLoadFilterSet) + XCTAssertFalse(mockDataActivities.started) + let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) + XCTAssertFalse(isMalicious) + XCTAssertTrue(mockDataActivities.stopped) + } + + func testIsMalicious() async { + PhishingDetectionPreferences.shared.isEnabled = true + let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) + XCTAssertTrue(isMalicious) + } + + func testIsNotMalicious() async { + PhishingDetectionPreferences.shared.isEnabled = true + let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://trusted.com")!) + XCTAssertFalse(isMalicious) + } +} From 4036618538e8909ffc85ca061ed4f3a7484cd8d8 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Thu, 5 Sep 2024 15:10:20 +0100 Subject: [PATCH 02/28] Cleanup tests --- .../PhishingDetectionIntegrationTests.swift | 160 +++++++----------- .../PhishingDetectionTests.swift | 1 + 2 files changed, 59 insertions(+), 102 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 9af201a33f..388af74fc3 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -32,10 +32,9 @@ class PhishingDetectionIntegrationTests: XCTestCase { @MainActor override func setUp() { - // disable GPC redirects + super.setUp() WebTrackingProtectionPreferences.shared.isGPCEnabled = false PhishingDetectionPreferences.shared.isEnabled = true - window = WindowsManager.openNewWindow(with: .none)! } @@ -43,134 +42,91 @@ class PhishingDetectionIntegrationTests: XCTestCase { override func tearDown() async throws { window.close() window = nil - WebTrackingProtectionPreferences.shared.isGPCEnabled = true + try await super.tearDown() } // MARK: - Tests - @MainActor - func testWhenPhishingNotDetected_tabIsNotMarkedPhishing() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - // load the test page - let url = URL(string: "http://privacy-test-pages.site/")! - _=await tab.setUrl(url, source: .link)?.result - - let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? true - XCTAssertFalse(isPhishingErrorPage) + @MainActor + func testPhishingNotDetected_tabIsNotMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/") + XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) } @MainActor - func testWhenPhishingDetected_tabIsMarkedPhishing() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - - // load fake phishing test page - let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! - _=await tab.setUrl(url, source: .link)?.result - - let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) + func testPhishingDetected_tabIsMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) } @MainActor - func testWhenPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - - // load fake phishing test page - let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! - _=await tab.setUrl(url, source: .link)?.result - var isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - XCTAssertTrue(isPhishingErrorPage) + func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + try await Task.sleep(nanoseconds: 1_000_000_000) - let url2 = URL(string: "http://broken.third-party.site/")! - _=await tab.setUrl(url2, source: .link)?.result + try await loadUrl("http://broken.third-party.site/") try await Task.sleep(nanoseconds: 1_000_000_000) - let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? true - isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? true - XCTAssertFalse(privacyInfoPhishing) - XCTAssertFalse(isPhishingErrorPage) + + XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) + XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) } @MainActor - func testWhenPhishingWarningClickedThrough_tabIsMarkedAsBypassed() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - - // load fake phishing test page - errorPageType = Phishing - let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! - _=await tab.setUrl(url, source: .link)?.result - var isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - var privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) - while tab.isLoading { + func testPhishingWarningClickedThrough_privacyInfoIsUpdated() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + + while tabViewModel.tab.isLoading { try await Task.sleep(nanoseconds: 1_000_000_000) } - // now visit site - let showAdvancedScript: String = "document.getElementsByClassName('Warning_advanced')[0].click()" - try? await tab.webView.evaluateJavaScript(showAdvancedScript) as Void? - let clickThroughScript: String = "document.getElementsByClassName('AdvancedInfo_visitSite')[0].click()" - try? await tab.webView.evaluateJavaScript(clickThroughScript) as Void? - try await Task.sleep(nanoseconds: 1_000_000_000) - isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) + + try await clickThroughPhishingWarning() + XCTAssertTrue(tabViewModel.tab.privacyInfo?.isPhishing ?? false) + XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) } @MainActor - func testWhenPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - - // load fake phishing test page - let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! - _=await tab.setUrl(url, source: .link)?.result - var isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - var privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) + func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + try await Task.sleep(nanoseconds: 1_000_000_000) - // we have special exceptions for DDG URLs, which previously caused a bug where phishing sites that quickly navigated to DDG caused a broken phishingState - let url2 = URL(string: "http://duckduckgo.com/")! - _=await tab.setUrl(url2, source: .link)?.result + try await loadUrl("http://duckduckgo.com/") try await Task.sleep(nanoseconds: 1_000_000_000) - privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) + + XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) + XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) + } + + @MainActor + func testPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-iframe-loader.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + } + + @MainActor + func testPhishingDetectedViaRedirectChain_tabIsMarkedPhishing() async throws { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) } + // MARK: - Helper Methods @MainActor - func testWhenPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - - // load fake phishing test page - let url = URL(string: "http://bad.third-party.site/security/badware/phishing-iframe-loader.html")! - _=await tab.setUrl(url, source: .link)?.result - let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) + private func loadUrl(_ urlString: String) async throws { + let url = URL(string: urlString)! + _ = await tabViewModel.tab.setUrl(url, source: .link)?.result } @MainActor - func testWhenPhishingDetectedViaRedirectChain_tabIsMarkedPhishing() async throws { - let tabViewModel = self.tabViewModel - let tab = tabViewModel.tab - - // load fake phishing test page with redirector - let url = URL(string: "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html")! - _=await tab.setUrl(url, source: .link)?.result - let isPhishingErrorPage = tab.url?.isPhishingErrorPage ?? false - let privacyInfoPhishing = tab.privacyInfo?.isPhishing ?? false - XCTAssertTrue(privacyInfoPhishing) - XCTAssertTrue(isPhishingErrorPage) + private func clickThroughPhishingWarning() async throws { + let showAdvancedScript = "document.getElementsByClassName('Warning_advanced')[0].click()" + try? await tabViewModel.tab.webView.evaluateJavaScript(showAdvancedScript) as Void? + + let clickThroughScript = "document.getElementsByClassName('AdvancedInfo_visitSite')[0].click()" + try? await tabViewModel.tab.webView.evaluateJavaScript(clickThroughScript) as Void? + + try await Task.sleep(nanoseconds: 1_000_000_000) } } diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift index 27292ae73d..6c0ca09bad 100644 --- a/UnitTests/PhishingDetection/PhishingDetectionTests.swift +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -50,6 +50,7 @@ final class PhishingDetectionTests: XCTestCase { } func testDidLoadAndStartDataActivities() async { + PhishingDetectionPreferences.shared.isEnabled = true XCTAssertTrue(mockDataActivities.started) } From 350b28df0d87b7ec639abd89530f300635a42507 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 12:00:49 +0100 Subject: [PATCH 03/28] Make integration tests more robust. --- .../PhishingDetectionIntegrationTests.swift | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 388af74fc3..db77c16a56 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -101,15 +101,40 @@ class PhishingDetectionIntegrationTests: XCTestCase { } @MainActor - func testPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-iframe-loader.html") + func testPhishingDetectedViaJSRedirectChain_tabIsMarkedPhishing() async throws { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + try await Task.sleep(nanoseconds: 1_000_000_000) + let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageHTMLDidRender) } @MainActor - func testPhishingDetectedViaRedirectChain_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") + func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + try await Task.sleep(nanoseconds: 1_000_000_000) + let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageHTMLDidRender) + } + + @MainActor + func testPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-iframe-loader.html") + try await Task.sleep(nanoseconds: 1_000_000_000) + let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageHTMLDidRender) + } + + @MainActor + func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { + for _ in 0..<10 { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + try await Task.sleep(nanoseconds: 1_000_000_000) + let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageHTMLDidRender) + } } // MARK: - Helper Methods @@ -119,6 +144,19 @@ class PhishingDetectionIntegrationTests: XCTestCase { _ = await tabViewModel.tab.setUrl(url, source: .link)?.result } + @MainActor + func errorPageHTMLRendered(inTab: Tab) async -> Bool { + return await withCheckedContinuation { continuation in + inTab.webView.evaluateJavaScript("document.documentElement.outerHTML") { (html, error) in + var containsPhishing = false + if let htmlString = html as? String { + containsPhishing = htmlString.contains("Error Page") + } + continuation.resume(returning: containsPhishing) + } + } + } + @MainActor private func clickThroughPhishingWarning() async throws { let showAdvancedScript = "document.getElementsByClassName('Warning_advanced')[0].click()" From e036f347b038facb52458af16bfd3f58a06f5fa8 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 14:56:24 +0100 Subject: [PATCH 04/28] Remove sleeps from tests. --- .../PhishingDetectionIntegrationTests.swift | 98 +++++++++++-------- 1 file changed, 56 insertions(+), 42 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index db77c16a56..bf13dd4257 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -26,6 +26,8 @@ import XCTest class PhishingDetectionIntegrationTests: XCTestCase { var window: NSWindow! + var cancellables: Set! + var tabViewModel: TabViewModel { (window.contentViewController as! MainViewController).browserTabViewController.tabViewModel! } @@ -36,12 +38,14 @@ class PhishingDetectionIntegrationTests: XCTestCase { WebTrackingProtectionPreferences.shared.isGPCEnabled = false PhishingDetectionPreferences.shared.isEnabled = true window = WindowsManager.openNewWindow(with: .none)! + cancellables = Set() } @MainActor override func tearDown() async throws { window.close() window = nil + cancellables = nil WebTrackingProtectionPreferences.shared.isGPCEnabled = true try await super.tearDown() } @@ -58,43 +62,33 @@ class PhishingDetectionIntegrationTests: XCTestCase { func testPhishingDetected_tabIsMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) } @MainActor func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) - try await Task.sleep(nanoseconds: 1_000_000_000) try await loadUrl("http://broken.third-party.site/") - try await Task.sleep(nanoseconds: 1_000_000_000) + try await waitForTabToFinishLoading() XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) } - @MainActor - func testPhishingWarningClickedThrough_privacyInfoIsUpdated() async throws { - try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - - while tabViewModel.tab.isLoading { - try await Task.sleep(nanoseconds: 1_000_000_000) - } - - try await clickThroughPhishingWarning() - XCTAssertTrue(tabViewModel.tab.privacyInfo?.isPhishing ?? false) - XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) - } - @MainActor func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) - try await Task.sleep(nanoseconds: 1_000_000_000) try await loadUrl("http://duckduckgo.com/") - try await Task.sleep(nanoseconds: 1_000_000_000) + try await waitForTabToFinishLoading() XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) @@ -104,56 +98,78 @@ class PhishingDetectionIntegrationTests: XCTestCase { func testPhishingDetectedViaJSRedirectChain_tabIsMarkedPhishing() async throws { try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - try await Task.sleep(nanoseconds: 1_000_000_000) - let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageHTMLDidRender) + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) } @MainActor func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - try await Task.sleep(nanoseconds: 1_000_000_000) - let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageHTMLDidRender) + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) } @MainActor func testPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { try await loadUrl("http://bad.third-party.site/security/badware/phishing-iframe-loader.html") - try await Task.sleep(nanoseconds: 1_000_000_000) - let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageHTMLDidRender) + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) } @MainActor func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { - for _ in 0..<10 { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - try await Task.sleep(nanoseconds: 1_000_000_000) - let errorPageHTMLDidRender = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageHTMLDidRender) + let urls = [ + "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", + "http://bad.third-party.site/security/badware/phishing-js-redirector.html", + "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", + "http://bad.third-party.site/security/badware/phishing-redirect/", + "http://bad.third-party.site/security/badware/phishing-redirect/302", + "http://bad.third-party.site/security/badware/phishing-redirect/js", + "http://bad.third-party.site/security/badware/phishing-redirect/meta", + "http://bad.third-party.site/security/badware/phishing-redirect/meta2" + ] + + for url in urls { + try await loadUrl(url) + XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false, "URL should be flagged as phishing: \(url)") + let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) + XCTAssertTrue(errorPageLoaded) } } // MARK: - Helper Methods + @MainActor private func loadUrl(_ urlString: String) async throws { - let url = URL(string: urlString)! + guard let url = URL(string: urlString) else { return } _ = await tabViewModel.tab.setUrl(url, source: .link)?.result } + @MainActor + func waitForTabToFinishLoading() async throws { + let loadingExpectation = expectation(description: "Tab finished loading") + Task { + while tabViewModel.tab.isLoading { + await Task.yield() + } + loadingExpectation.fulfill() + } + await fulfillment(of: [loadingExpectation], timeout: 5) + } + @MainActor func errorPageHTMLRendered(inTab: Tab) async -> Bool { - return await withCheckedContinuation { continuation in - inTab.webView.evaluateJavaScript("document.documentElement.outerHTML") { (html, error) in - var containsPhishing = false - if let htmlString = html as? String { - containsPhishing = htmlString.contains("Error Page") + while true { + let html = await withCheckedContinuation { continuation in + inTab.webView.evaluateJavaScript("document.documentElement.outerHTML") { (html, error) in + continuation.resume(returning: html) } - continuation.resume(returning: containsPhishing) } + if let htmlString = html as? String, htmlString.contains("Warning_advanced") { + return true + } + await Task.yield() } } @@ -164,7 +180,5 @@ class PhishingDetectionIntegrationTests: XCTestCase { let clickThroughScript = "document.getElementsByClassName('AdvancedInfo_visitSite')[0].click()" try? await tabViewModel.tab.webView.evaluateJavaScript(clickThroughScript) as Void? - - try await Task.sleep(nanoseconds: 1_000_000_000) } } From 0faf308ddfeb96a72cf69b3a5cd30563c4294c02 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 14:56:51 +0100 Subject: [PATCH 05/28] Fix redirect chains breaking error page navigation flows. --- .../Tab/TabExtensions/SpecialErrorPageTabExtension.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift index 699d1f7773..18cf0bcdb1 100644 --- a/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift @@ -134,7 +134,7 @@ extension SpecialErrorPageTabExtension: NavigationResponder { errorData = SpecialErrorData(kind: .phishing, domain: domain, eTldPlus1: tld.eTLDplus1(failingURL?.host)) if let errorURL = generateErrorPageURL(url) { _ = webView?.load(URLRequest(url: errorURL)) - return .cancel + return .none } } else { return handleMaliciousIframe(navigationAction: navigationAction) @@ -152,7 +152,7 @@ extension SpecialErrorPageTabExtension: NavigationResponder { if let errorURL = generateErrorPageURL(iframeTopUrl) { _ = webView?.load(URLRequest(url: errorURL)) - return .cancel + return .none } return .next From d90bf2481c3e4cbddbbaf152738a2e1fc5371ee3 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 15:10:51 +0100 Subject: [PATCH 06/28] Simplify - remove what should be in UI tests --- .../PhishingDetectionIntegrationTests.swift | 49 ------------------- 1 file changed, 49 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index bf13dd4257..3ca767f4f1 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -62,16 +62,12 @@ class PhishingDetectionIntegrationTests: XCTestCase { func testPhishingDetected_tabIsMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) } @MainActor func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) try await loadUrl("http://broken.third-party.site/") try await waitForTabToFinishLoading() @@ -84,8 +80,6 @@ class PhishingDetectionIntegrationTests: XCTestCase { func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) try await loadUrl("http://duckduckgo.com/") try await waitForTabToFinishLoading() @@ -94,27 +88,10 @@ class PhishingDetectionIntegrationTests: XCTestCase { XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) } - @MainActor - func testPhishingDetectedViaJSRedirectChain_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) - } - @MainActor func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) - } - - @MainActor - func testPhishingDetectedInIframe_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-iframe-loader.html") - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) } @MainActor @@ -133,8 +110,6 @@ class PhishingDetectionIntegrationTests: XCTestCase { for url in urls { try await loadUrl(url) XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false, "URL should be flagged as phishing: \(url)") - let errorPageLoaded = await errorPageHTMLRendered(inTab: tabViewModel.tab) - XCTAssertTrue(errorPageLoaded) } } @@ -157,28 +132,4 @@ class PhishingDetectionIntegrationTests: XCTestCase { } await fulfillment(of: [loadingExpectation], timeout: 5) } - - @MainActor - func errorPageHTMLRendered(inTab: Tab) async -> Bool { - while true { - let html = await withCheckedContinuation { continuation in - inTab.webView.evaluateJavaScript("document.documentElement.outerHTML") { (html, error) in - continuation.resume(returning: html) - } - } - if let htmlString = html as? String, htmlString.contains("Warning_advanced") { - return true - } - await Task.yield() - } - } - - @MainActor - private func clickThroughPhishingWarning() async throws { - let showAdvancedScript = "document.getElementsByClassName('Warning_advanced')[0].click()" - try? await tabViewModel.tab.webView.evaluateJavaScript(showAdvancedScript) as Void? - - let clickThroughScript = "document.getElementsByClassName('AdvancedInfo_visitSite')[0].click()" - try? await tabViewModel.tab.webView.evaluateJavaScript(clickThroughScript) as Void? - } } From f3b69544178707a8ef7ede732b88f605ef294ecc Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 15:36:09 +0100 Subject: [PATCH 07/28] Add feature disabled test case --- .../PhishingDetection/Mocks/PhishingDetectionMocks.swift | 4 +++- UnitTests/PhishingDetection/PhishingDetectionTests.swift | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift b/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift index 9cbdebf227..2c77a29553 100644 --- a/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift +++ b/UnitTests/PhishingDetection/Mocks/PhishingDetectionMocks.swift @@ -58,14 +58,16 @@ class MockFileStorageManager: FileStorageManager { final class MockPhishingDataActivitites: PhishingDetectionDataActivityHandling { var started: Bool = false - var stopped: Bool = false + var stopped: Bool = true func start() { started = true + stopped = false } func stop() { stopped = true + started = false } } diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift index 6c0ca09bad..2440429645 100644 --- a/UnitTests/PhishingDetection/PhishingDetectionTests.swift +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -87,4 +87,10 @@ final class PhishingDetectionTests: XCTestCase { let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://trusted.com")!) XCTAssertFalse(isMalicious) } + + func testIsMaliciousWithFeatureDisabled() async { + PhishingDetectionPreferences.shared.isEnabled = false + let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) + XCTAssertFalse(isMalicious) + } } From 8d7f5207085e5603bb6f6bce798e7abe77ded780 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 15:38:32 +0100 Subject: [PATCH 08/28] Swiftlint fix --- UnitTests/PhishingDetection/PhishingDetectionTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift index 2440429645..4d5578c4ff 100644 --- a/UnitTests/PhishingDetection/PhishingDetectionTests.swift +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -87,7 +87,7 @@ final class PhishingDetectionTests: XCTestCase { let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://trusted.com")!) XCTAssertFalse(isMalicious) } - + func testIsMaliciousWithFeatureDisabled() async { PhishingDetectionPreferences.shared.isEnabled = false let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) From e97623aaa152ee36a9ee02607abe8d913bea947d Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 15:58:22 +0100 Subject: [PATCH 09/28] Replace .none reload with .redirect --- .../Tab/TabExtensions/SpecialErrorPageTabExtension.swift | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift index 18cf0bcdb1..db37fdeb32 100644 --- a/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift @@ -128,13 +128,14 @@ extension SpecialErrorPageTabExtension: NavigationResponder { private func handleMaliciousURL(for navigationAction: NavigationAction, url: URL) -> NavigationActionPolicy? { let domain: String errorPageType = .phishing - if navigationAction.mainFrameTarget != nil { + if let mainFrameTarget = navigationAction.mainFrameTarget { failingURL = url domain = url.host ?? url.toString(decodePunycode: true, dropScheme: true, dropTrailingSlash: true) errorData = SpecialErrorData(kind: .phishing, domain: domain, eTldPlus1: tld.eTLDplus1(failingURL?.host)) if let errorURL = generateErrorPageURL(url) { - _ = webView?.load(URLRequest(url: errorURL)) - return .none + return .redirect(mainFrameTarget) { navigator in + navigator.load(URLRequest(url: errorURL)) + } } } else { return handleMaliciousIframe(navigationAction: navigationAction) From ec85c3f4a550937ae9b53f756065d01bd59e7fd1 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 15:58:36 +0100 Subject: [PATCH 10/28] Add ErrorPageTabExtension tests for phishing --- .../ErrorPageTabExtensionTest.swift | 71 ++++++++++++++++++- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift b/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift index 35d8a3eaa5..7f15a275ce 100644 --- a/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift +++ b/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift @@ -27,19 +27,24 @@ import SpecialErrorPages @testable import DuckDuckGo_Privacy_Browser final class ErrorPageTabExtensionTest: XCTestCase { - var mockWebViewPublisher: PassthroughSubject! var scriptPublisher: PassthroughSubject! var errorPageExtention: SpecialErrorPageTabExtension! var credentialCreator: MockCredentialCreator! + var phishingDetection: MockPhishingSiteDetector! + var phishingStateManager: PhishingTabStateManager! let errorURLString = "com.example.error" + let phishingURLString = "https://privacy-test-pages.site/security/phishing.html" override func setUpWithError() throws { mockWebViewPublisher = PassthroughSubject() scriptPublisher = PassthroughSubject() - credentialCreator = MockCredentialCreator() let featureFlagger = MockFeatureFlagger() - errorPageExtention = SpecialErrorPageTabExtension(webViewPublisher: mockWebViewPublisher, scriptsPublisher: scriptPublisher, urlCredentialCreator: credentialCreator, featureFlagger: featureFlagger, phishingDetector: MockPhishingSiteDetector(isMalicious: true), phishingStateManager: PhishingTabStateManager()) + credentialCreator = MockCredentialCreator() + phishingStateManager = PhishingTabStateManager() + phishingDetection = MockPhishingSiteDetector(isMalicious: true) + errorPageExtention = SpecialErrorPageTabExtension(webViewPublisher: mockWebViewPublisher, scriptsPublisher: scriptPublisher, urlCredentialCreator: credentialCreator, featureFlagger: featureFlagger, phishingDetector: phishingDetection, + phishingStateManager: phishingStateManager) } override func tearDownWithError() throws { @@ -318,6 +323,66 @@ final class ErrorPageTabExtensionTest: XCTestCase { XCTAssertNil(disposition) } + @MainActor func testWhenPhishingDetected_ThenPhishingErrorPageIsShown() async { + // GIVEN + let mockWebView = MockWKWebView(url: URL(string: phishingURLString)!) + let mainFrameNavigation = Navigation(identity: NavigationIdentity(nil), responders: ResponderChain(), state: .started, isCurrent: true) + let urlRequest = URLRequest(url: URL(string: phishingURLString)!) + let mainFrameTarget = FrameInfo(webView: nil, handle: FrameHandle(rawValue: 1 as UInt64)!, isMainFrame: true, url: URL(string: phishingURLString)!, securityOrigin: .empty) + let navigationAction = NavigationAction(request: urlRequest, navigationType: .custom(.userEnteredUrl), currentHistoryItemIdentity: nil, redirectHistory: [NavigationAction](), isUserInitiated: true, sourceFrame: FrameInfo(frame: WKFrameInfo()), targetFrame: mainFrameTarget, shouldDownload: false, mainFrameNavigation: mainFrameNavigation) + var preferences = NavigationPreferences(userAgent: "dummy", contentMode: .desktop, javaScriptEnabled: true) + errorPageExtention.webView = mockWebView + + // WHEN + let policy = await errorPageExtention.decidePolicy(for: navigationAction, preferences: &preferences) + + // THEN + XCTAssertEqual(policy.debugDescription, "redirect") + XCTAssertTrue(phishingStateManager.isShowingPhishingError) + } + + @MainActor func testWhenPhishingDetected_AndVisitSiteClicked_ThenNavigationProceeds() async { + // GIVEN + let mockWebView = MockWKWebView(url: URL(string: phishingURLString)!) + let mainFrameNavigation = Navigation(identity: NavigationIdentity(nil), responders: ResponderChain(), state: .started, isCurrent: true) + let urlRequest = URLRequest(url: URL(string: phishingURLString)!) + let mainFrameTarget = FrameInfo(webView: nil, handle: FrameHandle(rawValue: 1 as UInt64)!, isMainFrame: true, url: URL(string: phishingURLString)!, securityOrigin: .empty) + let navigationAction = NavigationAction(request: urlRequest, navigationType: .custom(.userEnteredUrl), currentHistoryItemIdentity: nil, redirectHistory: [NavigationAction](), isUserInitiated: true, sourceFrame: FrameInfo(frame: WKFrameInfo()), targetFrame: mainFrameTarget, shouldDownload: false, mainFrameNavigation: mainFrameNavigation) + var preferences = NavigationPreferences(userAgent: "dummy", contentMode: .desktop, javaScriptEnabled: true) + errorPageExtention.webView = mockWebView + _ = await errorPageExtention.decidePolicy(for: navigationAction, preferences: &preferences) + + // WHEN + errorPageExtention.visitSite() + let policy = await errorPageExtention.decidePolicy(for: navigationAction, preferences: &preferences) + + // THEN + XCTAssertEqual(policy.debugDescription, "next") + XCTAssertTrue(mockWebView.reloadCalled) + XCTAssertTrue(mockWebView.canGoBack) + XCTAssertFalse(phishingStateManager.isShowingPhishingError) + XCTAssertTrue(phishingStateManager.didBypassError) + } + + @MainActor func testWhenPhishingNotDetected_ThenNavigationProceeds() async { + // GIVEN + phishingDetection.isMalicious = false + let mockWebView = MockWKWebView(url: URL(string: phishingURLString)!) + let mainFrameNavigation = Navigation(identity: NavigationIdentity(nil), responders: ResponderChain(), state: .started, isCurrent: true) + let urlRequest = URLRequest(url: URL(string: phishingURLString)!) + let mainFrameTarget = FrameInfo(webView: nil, handle: FrameHandle(rawValue: 1 as UInt64)!, isMainFrame: true, url: URL(string: phishingURLString)!, securityOrigin: .empty) + let navigationAction = NavigationAction(request: urlRequest, navigationType: .custom(.userEnteredUrl), currentHistoryItemIdentity: nil, redirectHistory: [NavigationAction](), isUserInitiated: true, sourceFrame: FrameInfo(frame: WKFrameInfo()), targetFrame: mainFrameTarget, shouldDownload: false, mainFrameNavigation: mainFrameNavigation) + var preferences = NavigationPreferences(userAgent: "dummy", contentMode: .desktop, javaScriptEnabled: true) + errorPageExtention.webView = mockWebView + + // WHEN + let policy = await errorPageExtention.decidePolicy(for: navigationAction, preferences: &preferences) + + // THEN + XCTAssertEqual(policy.debugDescription, "next") + XCTAssertFalse(mockWebView.reloadCalled) + XCTAssertFalse(phishingStateManager.isShowingPhishingError) + } } class MockWKWebView: NSObject, ErrorPageTabExtensionNavigationDelegate { From 3923cf59eb92e3ebc6c6e505b2e0ad491388d6b2 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 16:01:50 +0100 Subject: [PATCH 11/28] Add DuckURLSchemeHandler tests --- .../DuckSchemeHandlerTests.swift | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift b/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift index cec7c92df1..ef91a91cd9 100644 --- a/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift +++ b/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift @@ -21,6 +21,7 @@ import XCTest import Common import BrowserServicesKit import Combine +import PhishingDetection final class DuckSchemeHandlerTests: XCTestCase { @@ -113,6 +114,57 @@ final class DuckSchemeHandlerTests: XCTestCase { XCTAssertTrue(configuration.urlSchemeHandler(forURLScheme: "duck") is DuckURLSchemeHandler) } + @MainActor + func testErrorPageSchemeHandlerSetsError() { + // Given + let urlString = "https://privacy-test-pages.site/security/badware/phishing.html" + let phishingUrl = URL(string: urlString)! + let encodedURL = URLTokenValidator.base64URLEncode(data: urlString.data(using: .utf8)!) + let token = URLTokenValidator.shared.generateToken(for: phishingUrl) + let errorURLString = "duck://error?reason=phishing&url=\(encodedURL)&token=\(token)" + let errorURL = URL(string: errorURLString)! + let handler = DuckURLSchemeHandler() + let webView = WKWebView() + let schemeTask = MockSchemeTask(request: URLRequest(url: errorURL)) + + // When + handler.webView(webView, start: schemeTask) + + // Then + let error = PhishingDetectionError.detected + let expectedError = NSError(domain: PhishingDetectionError.errorDomain, code: error.errorCode, userInfo: [ + NSURLErrorFailingURLErrorKey: phishingUrl, + NSLocalizedDescriptionKey: error.errorUserInfo[NSLocalizedDescriptionKey] ?? "Phishing detected" + ]) + XCTAssertEqual(schemeTask.error! as NSError, expectedError) + } + + @MainActor + func testErrorPageSchemeHandlerSetsError_WhenTokenInvalid() { + // Given + let urlString = "https://privacy-test-pages.site/security/badware/phishing.html" + let encodedURL = URLTokenValidator.base64URLEncode(data: urlString.data(using: .utf8)!) + let token = "ababababababababababab" + let errorURLString = "duck://error?reason=phishing&url=\(encodedURL)&token=\(token)" + let errorURL = URL(string: errorURLString)! + let handler = DuckURLSchemeHandler() + let webView = WKWebView() + let schemeTask = MockSchemeTask(request: URLRequest(url: errorURL)) + + // When + handler.webView(webView, start: schemeTask) + + // Then + let error = WKError.unknown + let expectedError = NSError(domain: "Unexpected Error", code: error.rawValue, userInfo: [ + NSURLErrorFailingURLErrorKey: "about:blank", + NSLocalizedDescriptionKey: "Unexpected Error" + ]) + XCTAssertEqual(schemeTask.error! as NSError, expectedError) + } + + @MainActor + class MockWebView: WKWebView { var lastURLRequest: URLRequest? var lastLoadedHTML: String? From b5e74f6077de8fd26f1a7bb0f38bdca7d7ec43c2 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 16:16:09 +0100 Subject: [PATCH 12/28] Simplify testDidNotLoadAndStartDataActivities_IfFeatureDisabled --- UnitTests/PhishingDetection/PhishingDetectionTests.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift index 4d5578c4ff..92e96a71e7 100644 --- a/UnitTests/PhishingDetection/PhishingDetectionTests.swift +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -63,16 +63,10 @@ final class PhishingDetectionTests: XCTestCase { func testDidNotLoadAndStartDataActivities_IfFeatureDisabled() async { PhishingDetectionPreferences.shared.isEnabled = false - mockDataActivities = MockPhishingDataActivitites() - mockFileStorageManager = MockFileStorageManager() - mockDataProvider = MockDataProvider() - dataStore = PhishingDetectionDataStore(dataProvider: mockDataProvider, fileStorageManager: mockFileStorageManager) phishingDetection = PhishingDetection(dataActivities: mockDataActivities, dataStore: dataStore, detector: mockDetector) XCTAssertFalse(mockDataProvider.didLoadHashPrefixes) XCTAssertFalse(mockDataProvider.didLoadFilterSet) XCTAssertFalse(mockDataActivities.started) - let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) - XCTAssertFalse(isMalicious) XCTAssertTrue(mockDataActivities.stopped) } From fa52d2254ce0890fb1dacd065850e9060c31bed8 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 16:16:59 +0100 Subject: [PATCH 13/28] Remove duplicate tests --- UnitTests/PhishingDetection/PhishingDetectionTests.swift | 7 ------- 1 file changed, 7 deletions(-) diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift index 92e96a71e7..2d60c72cab 100644 --- a/UnitTests/PhishingDetection/PhishingDetectionTests.swift +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -58,7 +58,6 @@ final class PhishingDetectionTests: XCTestCase { PhishingDetectionPreferences.shared.isEnabled = false let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) XCTAssertFalse(isMalicious) - XCTAssertTrue(mockDataActivities.stopped) } func testDidNotLoadAndStartDataActivities_IfFeatureDisabled() async { @@ -81,10 +80,4 @@ final class PhishingDetectionTests: XCTestCase { let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://trusted.com")!) XCTAssertFalse(isMalicious) } - - func testIsMaliciousWithFeatureDisabled() async { - PhishingDetectionPreferences.shared.isEnabled = false - let isMalicious = await phishingDetection.checkIsMaliciousIfEnabled(url: URL(string: "https://malicious.com")!) - XCTAssertFalse(isMalicious) - } } From e103d7e30fc002656e092d6062112cd8cee5faa1 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 16:24:11 +0100 Subject: [PATCH 14/28] Add privacyDashboardIntegrationTest for phishingInfo --- .../PrivacyDashboardIntegrationTests.swift | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/IntegrationTests/PrivacyDashboard/PrivacyDashboardIntegrationTests.swift b/IntegrationTests/PrivacyDashboard/PrivacyDashboardIntegrationTests.swift index 6af50b4681..bf2dcd63e7 100644 --- a/IntegrationTests/PrivacyDashboard/PrivacyDashboardIntegrationTests.swift +++ b/IntegrationTests/PrivacyDashboard/PrivacyDashboardIntegrationTests.swift @@ -86,4 +86,25 @@ class PrivacyDashboardIntegrationTests: XCTestCase { XCTAssertEqual(trackersCount2, 0) } + @MainActor + func testWhenPhishingDetected_phishingInfoUpdated() async throws { + let tabViewModel = self.tabViewModel + let tab = tabViewModel.tab + + let isPhishingPromise = tab.privacyInfoPublisher + .compactMap { + $0?.$isPhishing + } + .map { _ in true } + .timeout(10) + .first() + .promise() + // Load the test page + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + _ = await tab.setUrl(url, source: .link)?.result + + let isPhishing = try await isPhishingPromise.value + XCTAssertTrue(isPhishing) + } + } From bf578d99041ea933dd55a68ae10da76d203a329a Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 17:03:41 +0100 Subject: [PATCH 15/28] Remove duplicate PhishingDetection initialization --- UnitTests/PhishingDetection/PhishingDetectionTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/UnitTests/PhishingDetection/PhishingDetectionTests.swift b/UnitTests/PhishingDetection/PhishingDetectionTests.swift index 2d60c72cab..7712f4e605 100644 --- a/UnitTests/PhishingDetection/PhishingDetectionTests.swift +++ b/UnitTests/PhishingDetection/PhishingDetectionTests.swift @@ -62,7 +62,6 @@ final class PhishingDetectionTests: XCTestCase { func testDidNotLoadAndStartDataActivities_IfFeatureDisabled() async { PhishingDetectionPreferences.shared.isEnabled = false - phishingDetection = PhishingDetection(dataActivities: mockDataActivities, dataStore: dataStore, detector: mockDetector) XCTAssertFalse(mockDataProvider.didLoadHashPrefixes) XCTAssertFalse(mockDataProvider.didLoadFilterSet) XCTAssertFalse(mockDataActivities.started) From 25d455947969ffa4757b334c5f6b328f3678a97d Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 17:18:31 +0100 Subject: [PATCH 16/28] Test disable integration tests --- .../PhishingDetectionIntegrationTests.swift | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 3ca767f4f1..62498d23bb 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -52,66 +52,66 @@ class PhishingDetectionIntegrationTests: XCTestCase { // MARK: - Tests - @MainActor - func testPhishingNotDetected_tabIsNotMarkedPhishing() async throws { - try await loadUrl("http://privacy-test-pages.site/") - XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) - } - - @MainActor - func testPhishingDetected_tabIsMarkedPhishing() async throws { - try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - } - - @MainActor - func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { - try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - - try await loadUrl("http://broken.third-party.site/") - try await waitForTabToFinishLoading() - - XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) - XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) - } - - @MainActor - func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { - try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - - try await loadUrl("http://duckduckgo.com/") - try await waitForTabToFinishLoading() - - XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) - XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) - } - - @MainActor - func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) - } - - @MainActor - func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { - let urls = [ - "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", - "http://bad.third-party.site/security/badware/phishing-js-redirector.html", - "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", - "http://bad.third-party.site/security/badware/phishing-redirect/", - "http://bad.third-party.site/security/badware/phishing-redirect/302", - "http://bad.third-party.site/security/badware/phishing-redirect/js", - "http://bad.third-party.site/security/badware/phishing-redirect/meta", - "http://bad.third-party.site/security/badware/phishing-redirect/meta2" - ] - - for url in urls { - try await loadUrl(url) - XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false, "URL should be flagged as phishing: \(url)") - } - } +// @MainActor +// func testPhishingNotDetected_tabIsNotMarkedPhishing() async throws { +// try await loadUrl("http://privacy-test-pages.site/") +// XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) +// } +// +// @MainActor +// func testPhishingDetected_tabIsMarkedPhishing() async throws { +// try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") +// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) +// } +// +// @MainActor +// func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { +// try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") +// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) +// +// try await loadUrl("http://broken.third-party.site/") +// try await waitForTabToFinishLoading() +// +// XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) +// XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) +// } +// +// @MainActor +// func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { +// try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") +// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) +// +// try await loadUrl("http://duckduckgo.com/") +// try await waitForTabToFinishLoading() +// +// XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) +// XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) +// } +// +// @MainActor +// func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { +// try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") +// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) +// } +// +// @MainActor +// func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { +// let urls = [ +// "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", +// "http://bad.third-party.site/security/badware/phishing-js-redirector.html", +// "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", +// "http://bad.third-party.site/security/badware/phishing-redirect/", +// "http://bad.third-party.site/security/badware/phishing-redirect/302", +// "http://bad.third-party.site/security/badware/phishing-redirect/js", +// "http://bad.third-party.site/security/badware/phishing-redirect/meta", +// "http://bad.third-party.site/security/badware/phishing-redirect/meta2" +// ] +// +// for url in urls { +// try await loadUrl(url) +// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false, "URL should be flagged as phishing: \(url)") +// } +// } // MARK: - Helper Methods From 03b77e79162e039626a6054d562614519fb2c6b7 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 18:39:24 +0100 Subject: [PATCH 17/28] Revert .load and return .none instead of .redirect --- .../Tab/TabExtensions/SpecialErrorPageTabExtension.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift index db37fdeb32..18cf0bcdb1 100644 --- a/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/SpecialErrorPageTabExtension.swift @@ -128,14 +128,13 @@ extension SpecialErrorPageTabExtension: NavigationResponder { private func handleMaliciousURL(for navigationAction: NavigationAction, url: URL) -> NavigationActionPolicy? { let domain: String errorPageType = .phishing - if let mainFrameTarget = navigationAction.mainFrameTarget { + if navigationAction.mainFrameTarget != nil { failingURL = url domain = url.host ?? url.toString(decodePunycode: true, dropScheme: true, dropTrailingSlash: true) errorData = SpecialErrorData(kind: .phishing, domain: domain, eTldPlus1: tld.eTLDplus1(failingURL?.host)) if let errorURL = generateErrorPageURL(url) { - return .redirect(mainFrameTarget) { navigator in - navigator.load(URLRequest(url: errorURL)) - } + _ = webView?.load(URLRequest(url: errorURL)) + return .none } } else { return handleMaliciousIframe(navigationAction: navigationAction) From ee06b778f023eaf1a583e58d8a65918c4b195f5e Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 18:39:38 +0100 Subject: [PATCH 18/28] Rewrite integration tests to look at tab.error instead of url --- .../PhishingDetectionIntegrationTests.swift | 130 ++++++++++-------- 1 file changed, 70 insertions(+), 60 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 62498d23bb..8fbf372319 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -19,6 +19,7 @@ import Combine import Common import XCTest +import PhishingDetection @testable import DuckDuckGo_Privacy_Browser @@ -52,66 +53,75 @@ class PhishingDetectionIntegrationTests: XCTestCase { // MARK: - Tests -// @MainActor -// func testPhishingNotDetected_tabIsNotMarkedPhishing() async throws { -// try await loadUrl("http://privacy-test-pages.site/") -// XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) -// } -// -// @MainActor -// func testPhishingDetected_tabIsMarkedPhishing() async throws { -// try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") -// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) -// } -// -// @MainActor -// func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { -// try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") -// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) -// -// try await loadUrl("http://broken.third-party.site/") -// try await waitForTabToFinishLoading() -// -// XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) -// XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) -// } -// -// @MainActor -// func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { -// try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") -// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) -// -// try await loadUrl("http://duckduckgo.com/") -// try await waitForTabToFinishLoading() -// -// XCTAssertFalse(tabViewModel.tab.privacyInfo?.isPhishing ?? true) -// XCTAssertFalse(tabViewModel.tab.url?.isPhishingErrorPage ?? true) -// } -// -// @MainActor -// func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { -// try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") -// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false) -// } -// -// @MainActor -// func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { -// let urls = [ -// "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", -// "http://bad.third-party.site/security/badware/phishing-js-redirector.html", -// "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", -// "http://bad.third-party.site/security/badware/phishing-redirect/", -// "http://bad.third-party.site/security/badware/phishing-redirect/302", -// "http://bad.third-party.site/security/badware/phishing-redirect/js", -// "http://bad.third-party.site/security/badware/phishing-redirect/meta", -// "http://bad.third-party.site/security/badware/phishing-redirect/meta2" -// ] -// -// for url in urls { -// try await loadUrl(url) -// XCTAssertTrue(tabViewModel.tab.url?.isPhishingErrorPage ?? false, "URL should be flagged as phishing: \(url)") -// } -// } + @MainActor + func testPhishingNotDetected_tabIsNotMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/") + let tabErrorCode2 = tabViewModel.tab.error?.errorCode + XCTAssertNil(tabErrorCode2) + } + + @MainActor + func testPhishingDetected_tabIsMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + } + + @MainActor + func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + + try await loadUrl("http://broken.third-party.site/") + try await waitForTabToFinishLoading() + let tabErrorCode2 = tabViewModel.tab.error?.errorCode + XCTAssertNil(tabErrorCode2) + } + + @MainActor + func testPhishingDetectedThenDDGLoaded_tabIsNotMarkedPhishing() async throws { + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + + try await loadUrl("http://duckduckgo.com/") + try await waitForTabToFinishLoading() + let tabErrorCode2 = tabViewModel.tab.error?.errorCode + XCTAssertNil(tabErrorCode2) + } + + @MainActor + func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { + try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + } + + @MainActor + func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { + let urls = [ + "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", + "http://bad.third-party.site/security/badware/phishing-js-redirector.html", + "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", + "http://bad.third-party.site/security/badware/phishing-redirect/", + "http://bad.third-party.site/security/badware/phishing-redirect/302", + "http://bad.third-party.site/security/badware/phishing-redirect/js", + "http://bad.third-party.site/security/badware/phishing-redirect/meta", + "http://bad.third-party.site/security/badware/phishing-redirect/meta2" + ] + + for url in urls { + try await loadUrl(url) + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + } + } // MARK: - Helper Methods From 132192d98d4ac7119551a11ff6433acbf05821e9 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 6 Sep 2024 18:39:57 +0100 Subject: [PATCH 19/28] Refactor UnitTests to fix handling .load instead of .redirect. --- UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift b/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift index 7f15a275ce..d6f2d7ff79 100644 --- a/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift +++ b/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift @@ -337,7 +337,8 @@ final class ErrorPageTabExtensionTest: XCTestCase { let policy = await errorPageExtention.decidePolicy(for: navigationAction, preferences: &preferences) // THEN - XCTAssertEqual(policy.debugDescription, "redirect") + XCTAssertEqual(policy.debugDescription, "next") + XCTAssertTrue(mockWebView.loadCalled) XCTAssertTrue(phishingStateManager.isShowingPhishingError) } @@ -422,7 +423,7 @@ class MockWKWebView: NSObject, ErrorPageTabExtensionNavigationDelegate { func load(_ request: URLRequest) -> WKNavigation? { loadCalled = true - return .some(WKNavigation()) + return .none } } From ae0a79208b2d24aace353062a6127cb3834a5db5 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 10:10:25 +0100 Subject: [PATCH 20/28] Add extra checks to determine remote test failure. --- DuckDuckGo/PhishingDetection/PhishingDetection.swift | 4 ++++ .../PhishingDetectionIntegrationTests.swift | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/DuckDuckGo/PhishingDetection/PhishingDetection.swift b/DuckDuckGo/PhishingDetection/PhishingDetection.swift index 9c6688b24d..9f74e53356 100644 --- a/DuckDuckGo/PhishingDetection/PhishingDetection.swift +++ b/DuckDuckGo/PhishingDetection/PhishingDetection.swift @@ -133,6 +133,10 @@ public class PhishingDetection: PhishingSiteDetecting { let resolvedDataActivities = dataActivities ?? PhishingDetectionDataActivities(phishingDetectionDataProvider: resolvedDataProvider, updateManager: resolvedUpdateManager) return (resolvedDataStore, resolvedDetector, resolvedUpdateManager, resolvedDataActivities) } + + public func isEnabled() -> (Bool, Bool) { + return (featureFlagger.isFeatureOn(.phishingDetection), self.detectionPreferences.isEnabled) + } private func startUpdateTasksIfEnabled() { if featureFlagger.isFeatureOn(.phishingDetection), diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 8fbf372319..e6a4622869 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -63,6 +63,12 @@ class PhishingDetectionIntegrationTests: XCTestCase { @MainActor func testPhishingDetected_tabIsMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + let (featureFlagEnabled, preferencesEnabled) = PhishingDetection.shared.isEnabled() + XCTAssertTrue(featureFlagEnabled, "Internal feature flag should be enabled") + XCTAssertTrue(preferencesEnabled, "Preferences setting should be enabled") + let phishingDetected = await PhishingDetection.shared.checkIsMaliciousIfEnabled(url: url) + XCTAssertTrue(phishingDetected, "PhishingDetection library should return malicious for \(url)") try await waitForTabToFinishLoading() let tabErrorCode = tabViewModel.tab.error?.errorCode XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) From c49e0bbedae0d69544940ff8089cf4ad7f845b75 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 10:35:39 +0100 Subject: [PATCH 21/28] Update featureFlagger references. --- DuckDuckGo.xcodeproj/project.pbxproj | 2 -- DuckDuckGo/PhishingDetection/PhishingDetection.swift | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 70e3658fd7..4f152eb2ad 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -4723,7 +4723,6 @@ EE7295E32A545B9A008C0991 /* NetworkProtection in Frameworks */, 9807F645278CA16F00E1547B /* BrowserServicesKit in Frameworks */, D6BC8AC62C5A95AA0025375B /* DuckPlayer in Frameworks */, - CDE248A02C821DF400F9399D /* PhishingDetection in Frameworks */, 987799ED299998B1005D8EB6 /* Bookmarks in Frameworks */, 1E950E3F2912A10D0051A99B /* ContentBlocking in Frameworks */, 31A3A4E32B0C115F0021063C /* DataBrokerProtection in Frameworks */, @@ -5286,7 +5285,6 @@ isa = PBXGroup; children = ( 9D9DE5712C63A96400D20B15 /* AppKitExtensions */, - CDE2489E2C821DE800F9399D /* BrowserServicesKit */, 7B9167A82C09E88800322310 /* AppLauncher */, 378E279D2970217400FCADA2 /* BuildToolPlugins */, 3192A2702A4C4E330084EA89 /* DataBrokerProtection */, diff --git a/DuckDuckGo/PhishingDetection/PhishingDetection.swift b/DuckDuckGo/PhishingDetection/PhishingDetection.swift index 6f0106eb9f..050aa1919e 100644 --- a/DuckDuckGo/PhishingDetection/PhishingDetection.swift +++ b/DuckDuckGo/PhishingDetection/PhishingDetection.swift @@ -135,7 +135,7 @@ public class PhishingDetection: PhishingSiteDetecting { } public func isEnabled() -> (Bool, Bool) { - return (featureFlagger.isFeatureOn(.phishingDetection), self.detectionPreferences.isEnabled) + return (featureFlagger.isFeatureOn(.phishingDetectionErrorPage), self.detectionPreferences.isEnabled) } private func startUpdateTasksIfEnabled() { From 7558e356d5938d7cfd8401946eefcb7a47b2eefd Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 11:08:38 +0100 Subject: [PATCH 22/28] Inject always-true feature flagger into PhishingDetection --- .../PhishingDetection/PhishingDetection.swift | 10 ++++--- .../PhishingDetectionIntegrationTests.swift | 28 ++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/DuckDuckGo/PhishingDetection/PhishingDetection.swift b/DuckDuckGo/PhishingDetection/PhishingDetection.swift index 050aa1919e..3d0f12c67d 100644 --- a/DuckDuckGo/PhishingDetection/PhishingDetection.swift +++ b/DuckDuckGo/PhishingDetection/PhishingDetection.swift @@ -105,6 +105,12 @@ public class PhishingDetection: PhishingSiteDetecting { ) } + convenience init(featureFlagger: FeatureFlagger) { + self.init( + dataStore: nil, detector: nil, dataActivities: nil, featureFlagger: featureFlagger + ) + } + private static func resolveDependencies( revision: Int, filterSetURL: URL, @@ -133,10 +139,6 @@ public class PhishingDetection: PhishingSiteDetecting { let resolvedDataActivities = dataActivities ?? PhishingDetectionDataActivities(phishingDetectionDataProvider: resolvedDataProvider, updateManager: resolvedUpdateManager) return (resolvedDataStore, resolvedDetector, resolvedUpdateManager, resolvedDataActivities) } - - public func isEnabled() -> (Bool, Bool) { - return (featureFlagger.isFeatureOn(.phishingDetectionErrorPage), self.detectionPreferences.isEnabled) - } private func startUpdateTasksIfEnabled() { if featureFlagger.isFeatureOn(.phishingDetectionErrorPage), diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index e6a4622869..c299e9f098 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -19,6 +19,7 @@ import Combine import Common import XCTest +import BrowserServicesKit import PhishingDetection @testable import DuckDuckGo_Privacy_Browser @@ -28,17 +29,20 @@ class PhishingDetectionIntegrationTests: XCTestCase { var window: NSWindow! var cancellables: Set! - - var tabViewModel: TabViewModel { - (window.contentViewController as! MainViewController).browserTabViewController.tabViewModel! - } + var phishingDetector: PhishingSiteDetecting! + var tab: Tab! + var tabViewModel: TabViewModel! @MainActor override func setUp() { super.setUp() WebTrackingProtectionPreferences.shared.isGPCEnabled = false PhishingDetectionPreferences.shared.isEnabled = true - window = WindowsManager.openNewWindow(with: .none)! + let featureFlagger = MockFeatureFlagger() + phishingDetector = PhishingDetection(featureFlagger: featureFlagger) + tab = Tab(content: .none, phishingDetector: phishingDetector) + tabViewModel = TabViewModel(tab: tab) + window = WindowsManager.openNewWindow(with: tab)! cancellables = Set() } @@ -47,6 +51,9 @@ class PhishingDetectionIntegrationTests: XCTestCase { window.close() window = nil cancellables = nil + phishingDetector = nil + tab = nil + tabViewModel = nil WebTrackingProtectionPreferences.shared.isGPCEnabled = true try await super.tearDown() } @@ -64,11 +71,6 @@ class PhishingDetectionIntegrationTests: XCTestCase { func testPhishingDetected_tabIsMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! - let (featureFlagEnabled, preferencesEnabled) = PhishingDetection.shared.isEnabled() - XCTAssertTrue(featureFlagEnabled, "Internal feature flag should be enabled") - XCTAssertTrue(preferencesEnabled, "Preferences setting should be enabled") - let phishingDetected = await PhishingDetection.shared.checkIsMaliciousIfEnabled(url: url) - XCTAssertTrue(phishingDetected, "PhishingDetection library should return malicious for \(url)") try await waitForTabToFinishLoading() let tabErrorCode = tabViewModel.tab.error?.errorCode XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) @@ -149,3 +151,9 @@ class PhishingDetectionIntegrationTests: XCTestCase { await fulfillment(of: [loadingExpectation], timeout: 5) } } + +class MockFeatureFlagger: FeatureFlagger { + func isFeatureOn(forProvider: F) -> Bool where F: BrowserServicesKit.FeatureFlagSourceProviding { + return true + } +} From 9fe5de9873c154297875f26436bb62d115176409 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 11:47:06 +0100 Subject: [PATCH 23/28] Inject PrivacyConfigManager --- .../PhishingDetection/PhishingDetection.swift | 43 +++++++++++++------ .../PhishingDetectionIntegrationTests.swift | 2 +- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/DuckDuckGo/PhishingDetection/PhishingDetection.swift b/DuckDuckGo/PhishingDetection/PhishingDetection.swift index 3d0f12c67d..a52e184e1e 100644 --- a/DuckDuckGo/PhishingDetection/PhishingDetection.swift +++ b/DuckDuckGo/PhishingDetection/PhishingDetection.swift @@ -38,7 +38,7 @@ public class PhishingDetection: PhishingSiteDetecting { private var detectionPreferences: PhishingDetectionPreferences private var dataStore: PhishingDetectionDataSaving private var featureFlagger: FeatureFlagger - private var config: PrivacyConfiguration + private var configManager: PrivacyConfigurationManaging private var cancellable: AnyCancellable? private let revision: Int private let filterSetURL: URL @@ -59,7 +59,8 @@ public class PhishingDetection: PhishingSiteDetecting { updateManager: PhishingDetectionUpdateManaging? = nil, dataActivities: PhishingDetectionDataActivityHandling? = nil, detectionPreferences: PhishingDetectionPreferences = PhishingDetectionPreferences.shared, - featureFlagger: FeatureFlagger? = nil + featureFlagger: FeatureFlagger? = nil, + configManager: PrivacyConfigurationManaging? = nil ) { self.revision = revision self.filterSetURL = filterSetURL @@ -79,15 +80,16 @@ public class PhishingDetection: PhishingSiteDetecting { dataStore: dataStore, detector: detector, updateManager: updateManager, - dataActivities: dataActivities + dataActivities: dataActivities, + configManager: configManager ) self.dataStore = resolvedDependencies.dataStore self.detector = resolvedDependencies.detector self.updateManager = resolvedDependencies.updateManager self.dataActivities = resolvedDependencies.dataActivities + self.configManager = resolvedDependencies.configManager self.detectionPreferences = detectionPreferences - self.config = AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager.privacyConfig self.startUpdateTasksIfEnabled() self.setupBindings() @@ -96,18 +98,28 @@ public class PhishingDetection: PhishingSiteDetecting { convenience init( dataActivities: PhishingDetectionDataActivityHandling, dataStore: PhishingDetectionDataSaving, - detector: PhishingDetecting + detector: PhishingDetecting, + configManager: PrivacyConfigurationManaging = AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager ) { self.init( - dataStore: dataStore, - detector: detector, - dataActivities: dataActivities + dataStore: dataStore, detector: detector, dataActivities: dataActivities, + detectionPreferences: PhishingDetectionPreferences.shared, + featureFlagger: NSApp.delegateTyped.featureFlagger, + configManager: configManager ) } - convenience init(featureFlagger: FeatureFlagger) { + convenience init(featureFlagger: FeatureFlagger, configManager: PrivacyConfigurationManaging) { self.init( - dataStore: nil, detector: nil, dataActivities: nil, featureFlagger: featureFlagger + detectionClient: PhishingDetectionAPIClient(), + dataProvider: nil, + dataStore: nil, + detector: nil, + updateManager: nil, + dataActivities: nil, + detectionPreferences: PhishingDetectionPreferences.shared, + featureFlagger: featureFlagger, + configManager: configManager ) } @@ -122,8 +134,9 @@ public class PhishingDetection: PhishingSiteDetecting { dataStore: PhishingDetectionDataSaving?, detector: PhishingDetecting?, updateManager: PhishingDetectionUpdateManaging?, - dataActivities: PhishingDetectionDataActivityHandling? - ) -> (dataStore: PhishingDetectionDataSaving, detector: PhishingDetecting, updateManager: PhishingDetectionUpdateManaging, dataActivities: PhishingDetectionDataActivityHandling) { + dataActivities: PhishingDetectionDataActivityHandling?, + configManager: PrivacyConfigurationManaging? + ) -> (dataStore: PhishingDetectionDataSaving, detector: PhishingDetecting, updateManager: PhishingDetectionUpdateManaging, dataActivities: PhishingDetectionDataActivityHandling, configManager: PrivacyConfigurationManaging) { let resolvedDataProvider = dataProvider ?? PhishingDetectionDataProvider( revision: revision, @@ -137,7 +150,9 @@ public class PhishingDetection: PhishingSiteDetecting { let resolvedDetector = detector ?? PhishingDetector(apiClient: detectionClient, dataStore: resolvedDataStore) let resolvedUpdateManager = updateManager ?? PhishingDetectionUpdateManager(client: detectionClient, dataStore: resolvedDataStore) let resolvedDataActivities = dataActivities ?? PhishingDetectionDataActivities(phishingDetectionDataProvider: resolvedDataProvider, updateManager: resolvedUpdateManager) - return (resolvedDataStore, resolvedDetector, resolvedUpdateManager, resolvedDataActivities) + let resolvedConfigManager = configManager ?? AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager + + return (resolvedDataStore, resolvedDetector, resolvedUpdateManager, resolvedDataActivities, resolvedConfigManager) } private func startUpdateTasksIfEnabled() { @@ -154,7 +169,7 @@ public class PhishingDetection: PhishingSiteDetecting { } public func checkIsMaliciousIfEnabled(url: URL) async -> Bool { - if config.isFeature(.phishingDetection, enabledForDomain: url.host), + if configManager.privacyConfig.isFeature(.phishingDetection, enabledForDomain: url.host), detectionPreferences.isEnabled { return await detector.isMalicious(url: url) } else { diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index c299e9f098..e3e809b579 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -39,7 +39,7 @@ class PhishingDetectionIntegrationTests: XCTestCase { WebTrackingProtectionPreferences.shared.isGPCEnabled = false PhishingDetectionPreferences.shared.isEnabled = true let featureFlagger = MockFeatureFlagger() - phishingDetector = PhishingDetection(featureFlagger: featureFlagger) + phishingDetector = PhishingDetection(featureFlagger: featureFlagger, configManager: MockPrivacyConfigurationManager()) tab = Tab(content: .none, phishingDetector: phishingDetector) tabViewModel = TabViewModel(tab: tab) window = WindowsManager.openNewWindow(with: tab)! From 494c11c95f0ca0907cfdc2e1338ab6f1ad110be9 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 12:09:37 +0100 Subject: [PATCH 24/28] Split redirect chain tests into two separate test cases --- .../PhishingDetection/PhishingDetection.swift | 13 +++++-------- .../PhishingDetectionIntegrationTests.swift | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/DuckDuckGo/PhishingDetection/PhishingDetection.swift b/DuckDuckGo/PhishingDetection/PhishingDetection.swift index a52e184e1e..365cdf0864 100644 --- a/DuckDuckGo/PhishingDetection/PhishingDetection.swift +++ b/DuckDuckGo/PhishingDetection/PhishingDetection.swift @@ -68,6 +68,7 @@ public class PhishingDetection: PhishingSiteDetecting { self.hashPrefixURL = hashPrefixURL self.hashPrefixDataSHA = hashPrefixDataSHA self.featureFlagger = featureFlagger ?? NSApp.delegateTyped.featureFlagger + self.configManager = configManager ?? AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager let resolvedDependencies = PhishingDetection.resolveDependencies( revision: revision, @@ -80,15 +81,13 @@ public class PhishingDetection: PhishingSiteDetecting { dataStore: dataStore, detector: detector, updateManager: updateManager, - dataActivities: dataActivities, - configManager: configManager + dataActivities: dataActivities ) self.dataStore = resolvedDependencies.dataStore self.detector = resolvedDependencies.detector self.updateManager = resolvedDependencies.updateManager self.dataActivities = resolvedDependencies.dataActivities - self.configManager = resolvedDependencies.configManager self.detectionPreferences = detectionPreferences self.startUpdateTasksIfEnabled() @@ -134,9 +133,8 @@ public class PhishingDetection: PhishingSiteDetecting { dataStore: PhishingDetectionDataSaving?, detector: PhishingDetecting?, updateManager: PhishingDetectionUpdateManaging?, - dataActivities: PhishingDetectionDataActivityHandling?, - configManager: PrivacyConfigurationManaging? - ) -> (dataStore: PhishingDetectionDataSaving, detector: PhishingDetecting, updateManager: PhishingDetectionUpdateManaging, dataActivities: PhishingDetectionDataActivityHandling, configManager: PrivacyConfigurationManaging) { + dataActivities: PhishingDetectionDataActivityHandling? + ) -> (dataStore: PhishingDetectionDataSaving, detector: PhishingDetecting, updateManager: PhishingDetectionUpdateManaging, dataActivities: PhishingDetectionDataActivityHandling) { let resolvedDataProvider = dataProvider ?? PhishingDetectionDataProvider( revision: revision, @@ -150,9 +148,8 @@ public class PhishingDetection: PhishingSiteDetecting { let resolvedDetector = detector ?? PhishingDetector(apiClient: detectionClient, dataStore: resolvedDataStore) let resolvedUpdateManager = updateManager ?? PhishingDetectionUpdateManager(client: detectionClient, dataStore: resolvedDataStore) let resolvedDataActivities = dataActivities ?? PhishingDetectionDataActivities(phishingDetectionDataProvider: resolvedDataProvider, updateManager: resolvedUpdateManager) - let resolvedConfigManager = configManager ?? AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager - return (resolvedDataStore, resolvedDetector, resolvedUpdateManager, resolvedDataActivities, resolvedConfigManager) + return (resolvedDataStore, resolvedDetector, resolvedUpdateManager, resolvedDataActivities) } private func startUpdateTasksIfEnabled() { diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index e3e809b579..7888609477 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -111,12 +111,24 @@ class PhishingDetectionIntegrationTests: XCTestCase { } @MainActor - func testPhishingDetectedRepeatedRedirectChains_tabIsMarkedPhishing() async throws { + func testPhishingDetectedRepeatedClientRedirectChains_tabIsMarkedPhishing() async throws { let urls = [ "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", "http://bad.third-party.site/security/badware/phishing-js-redirector.html", "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", - "http://bad.third-party.site/security/badware/phishing-redirect/", + ] + + for url in urls { + try await loadUrl(url) + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + } + } + + @MainActor + func testPhishingDetectedRepeatedServerRedirectChains_tabIsMarkedPhishing() async throws { + let urls = [ "http://bad.third-party.site/security/badware/phishing-redirect/302", "http://bad.third-party.site/security/badware/phishing-redirect/js", "http://bad.third-party.site/security/badware/phishing-redirect/meta", From 7a07edc442949605df6d3256ab1954e6913b839b Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 12:27:49 +0100 Subject: [PATCH 25/28] Remove flaky tests- repeatedRedirectChains Too many navigation events in short succession. --- .../PhishingDetectionIntegrationTests.swift | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 7888609477..146512bda6 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -110,39 +110,6 @@ class PhishingDetectionIntegrationTests: XCTestCase { XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) } - @MainActor - func testPhishingDetectedRepeatedClientRedirectChains_tabIsMarkedPhishing() async throws { - let urls = [ - "http://bad.third-party.site/security/badware/phishing-js-redirector-helper.html", - "http://bad.third-party.site/security/badware/phishing-js-redirector.html", - "http://bad.third-party.site/security/badware/phishing-meta-redirect.html", - ] - - for url in urls { - try await loadUrl(url) - try await waitForTabToFinishLoading() - let tabErrorCode = tabViewModel.tab.error?.errorCode - XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) - } - } - - @MainActor - func testPhishingDetectedRepeatedServerRedirectChains_tabIsMarkedPhishing() async throws { - let urls = [ - "http://bad.third-party.site/security/badware/phishing-redirect/302", - "http://bad.third-party.site/security/badware/phishing-redirect/js", - "http://bad.third-party.site/security/badware/phishing-redirect/meta", - "http://bad.third-party.site/security/badware/phishing-redirect/meta2" - ] - - for url in urls { - try await loadUrl(url) - try await waitForTabToFinishLoading() - let tabErrorCode = tabViewModel.tab.error?.errorCode - XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) - } - } - // MARK: - Helper Methods @MainActor From ee19078a54b1da636c75afbf9033ca49107a0f17 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 12:42:21 +0100 Subject: [PATCH 26/28] Replace HTTP redirect chain test. --- .../PhishingDetection/PhishingDetectionIntegrationTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 146512bda6..56cf9f338e 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -104,7 +104,7 @@ class PhishingDetectionIntegrationTests: XCTestCase { @MainActor func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") + try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/302") try await waitForTabToFinishLoading() let tabErrorCode = tabViewModel.tab.error?.errorCode XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) From fdef65374971e35e050b4826891a964d8750c762 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 12:57:16 +0100 Subject: [PATCH 27/28] Use privacy-test-pages.site as test origin. --- .../PhishingDetectionIntegrationTests.swift | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index 56cf9f338e..b68d27cebb 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -104,12 +104,45 @@ class PhishingDetectionIntegrationTests: XCTestCase { @MainActor func testPhishingDetectedViaHTTPRedirectChain_tabIsMarkedPhishing() async throws { - try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/302") + try await loadUrl("http://bad.third-party.site/security/badware/phishing-redirect/") try await waitForTabToFinishLoading() let tabErrorCode = tabViewModel.tab.error?.errorCode XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) } + @MainActor + func testPhishingDetectedRepeatedClientRedirectChains_tabIsMarkedPhishing() async throws { + let urls = [ + "http://privacy-test-pages.site/security/badware/phishing-js-redirector-helper.html", + "http://privacy-test-pages.site/security/badware/phishing-js-redirector.html", + "http://privacy-test-pages.site/security/badware/phishing-meta-redirect.html", + ] + + for url in urls { + try await loadUrl(url) + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + } + } + + @MainActor + func testPhishingDetectedRepeatedServerRedirectChains_tabIsMarkedPhishing() async throws { + let urls = [ + "http://privacy-test-pages.site/security/badware/phishing-redirect/302", + "http://privacy-test-pages.site/security/badware/phishing-redirect/js", + "http://privacy-test-pages.site/security/badware/phishing-redirect/meta", + "http://privacy-test-pages.site/security/badware/phishing-redirect/meta2" + ] + + for url in urls { + try await loadUrl(url) + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) + } + } + // MARK: - Helper Methods @MainActor From 0b89b223d72bdf2a02776a90adeaa35bfce2a7d8 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Mon, 9 Sep 2024 15:08:55 +0100 Subject: [PATCH 28/28] Add feature disabled test --- .../PhishingDetectionIntegrationTests.swift | 10 ++++++++++ .../DuckSchemeHandler/DuckSchemeHandlerTests.swift | 2 ++ 2 files changed, 12 insertions(+) diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index b68d27cebb..e38e10ffa4 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -76,6 +76,16 @@ class PhishingDetectionIntegrationTests: XCTestCase { XCTAssertEqual(tabErrorCode, PhishingDetectionError.detected.errorCode) } + @MainActor + func testFeatureDisabledAndPhishingDetection_tabIsNotMarkedPhishing() async throws { + PhishingDetectionPreferences.shared.isEnabled = false + try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") + let url = URL(string: "http://privacy-test-pages.site/security/badware/phishing.html")! + try await waitForTabToFinishLoading() + let tabErrorCode = tabViewModel.tab.error?.errorCode + XCTAssertNil(tabErrorCode) + } + @MainActor func testPhishingDetectedThenNotDetected_tabIsNotMarkedPhishing() async throws { try await loadUrl("http://privacy-test-pages.site/security/badware/phishing.html") diff --git a/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift b/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift index ef91a91cd9..7e28a5335c 100644 --- a/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift +++ b/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift @@ -136,6 +136,7 @@ final class DuckSchemeHandlerTests: XCTestCase { NSURLErrorFailingURLErrorKey: phishingUrl, NSLocalizedDescriptionKey: error.errorUserInfo[NSLocalizedDescriptionKey] ?? "Phishing detected" ]) + XCTAssertNotNil(schemeTask.error) XCTAssertEqual(schemeTask.error! as NSError, expectedError) } @@ -160,6 +161,7 @@ final class DuckSchemeHandlerTests: XCTestCase { NSURLErrorFailingURLErrorKey: "about:blank", NSLocalizedDescriptionKey: "Unexpected Error" ]) + XCTAssertNotNil(schemeTask.error) XCTAssertEqual(schemeTask.error! as NSError, expectedError) }