From 9f8ebcf5b27546f883d808d9d1b15dbb24e9a9d6 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 29 Nov 2024 16:57:22 +0100 Subject: [PATCH 01/15] Add Detection Logic --- .../SpecialErrorPageActionHandler.swift | 3 +- ...rPageNavigationHandler+MaliciousSite.swift | 64 +++++++++++-- .../SpecialErrorPageNavigationHandler.swift | 91 +++++++++++++------ DuckDuckGo/TabViewController.swift | 31 ++++--- 4 files changed, 141 insertions(+), 48 deletions(-) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift index 2d12540cde..d939be97cf 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift @@ -18,6 +18,7 @@ // import Foundation +import SpecialErrorPages /// A type that defines actions for handling special error pages. /// @@ -26,7 +27,7 @@ import Foundation /// advanced information related to the error. protocol SpecialErrorPageActionHandler { /// Handles the action of navigating to the site associated with the error page - func visitSite() + func visitSite(url: URL, errorData: SpecialErrorData) /// Handles the action of leaving the site associated with the error page func leaveSite() diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index e49ea0e215..448cb61f3f 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -24,8 +24,13 @@ import SpecialErrorPages import WebKit enum MaliciousSiteProtectionNavigationResult: Equatable { - case navigationHandled(SpecialErrorModel) + case navigationHandled(NavigationType) case navigationNotHandled + + enum NavigationType: Equatable { + case mainFrame(SpecialErrorData) + case iFrame(maliciousURL: URL, error: SpecialErrorData) + } } protocol MaliciousSiteProtectionNavigationHandling: AnyObject { @@ -41,7 +46,9 @@ protocol MaliciousSiteProtectionNavigationHandling: AnyObject { final class MaliciousSiteProtectionNavigationHandler { private let maliciousSiteProtectionManager: MaliciousSiteDetecting private let storageCache: StorageCache - + private var maliciousURLExemptions: [URL: MaliciousSiteProtection.ThreatKind] = [:] + private var bypassedMaliciousSiteThreatKind: ThreatKind? + init( maliciousSiteProtectionManager: MaliciousSiteDetecting = MaliciousSiteProtectionManager(), storageCache: StorageCache = AppDependencyProvider.shared.storageCache @@ -57,9 +64,34 @@ extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavig @MainActor func handleMaliciousSiteProtectionNavigation(for navigationAction: WKNavigationAction, webView: WKWebView) async -> MaliciousSiteProtectionNavigationResult { - // Implement logic to use `maliciousSiteProtectionManager.evaluate(url)` - // Return navigationNotHandled for the time being - return .navigationNotHandled + + guard let url = navigationAction.request.url else { + return .navigationNotHandled + } + + if let aboutBlankURL = URL(string: "about:blank"), url == aboutBlankURL { + return .navigationNotHandled + } + + handleMaliciousExemptions(for: navigationAction.navigationType, url: url) + + guard !shouldBypassMaliciousSiteProtection(for: url) else { + return .navigationNotHandled + } + + guard let threatKind = await maliciousSiteProtectionManager.evaluate(url) else { + return .navigationNotHandled + } + + if navigationAction.isTargetingMainFrame { + let errorData = SpecialErrorData.maliciousSite(kind: threatKind, url: url) + return .navigationHandled(.mainFrame(errorData)) + } else { + // Extract the URL of the source frame (the iframe) that initiated the navigation action + let iFrameTopURL = navigationAction.sourceFrame.safeRequest?.url ?? url + let errorData = SpecialErrorData.maliciousSite(kind: threatKind, url: iFrameTopURL) + return .navigationHandled(.iFrame(maliciousURL: url, error: errorData)) + } } } @@ -68,7 +100,10 @@ extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavig extension MaliciousSiteProtectionNavigationHandler: SpecialErrorPageActionHandler { - func visitSite() { + func visitSite(url: URL, errorData: SpecialErrorData) { + maliciousURLExemptions[url] = errorData.threatKind + bypassedMaliciousSiteThreatKind = errorData.threatKind + // Fire Pixel } @@ -81,3 +116,20 @@ extension MaliciousSiteProtectionNavigationHandler: SpecialErrorPageActionHandle } } + +// MARK: - Private + +private extension MaliciousSiteProtectionNavigationHandler { + + func handleMaliciousExemptions(for navigationType: WKNavigationType, url: URL) { + if let threatKind = bypassedMaliciousSiteThreatKind, navigationType == .other { + maliciousURLExemptions[url] = threatKind + } + bypassedMaliciousSiteThreatKind = maliciousURLExemptions[url] + } + + func shouldBypassMaliciousSiteProtection(for url: URL) -> Bool { + bypassedMaliciousSiteThreatKind != .none || url.isDuckDuckGo || url.isDuckURLScheme + } + +} diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index 57422c2610..cb750ae495 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -27,7 +27,6 @@ typealias SpecialErrorPageManaging = SpecialErrorPageContextHandling & WebViewNa final class SpecialErrorPageNavigationHandler: SpecialErrorPageContextHandling { private var webView: WKWebView? private(set) var errorData: SpecialErrorData? - private var errorPageType: SpecialErrorKind? private(set) var isSpecialErrorPageVisible = false private(set) var failedURL: URL? private weak var userScript: SpecialErrorPageUserScript? @@ -63,14 +62,18 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { return await MainActor.run { switch result { - case let .navigationHandled(model): + case let .navigationHandled(.mainFrame(error)): var request = navigationAction.request - request.url = model.url - failedURL = model.url - errorData = model.errorData - errorPageType = .phishing + request.url = error.url + failedURL = error.url + errorData = error loadSpecialErrorPage(request: request) return true + case let .navigationHandled(.iFrame(maliciousURL, error)): + failedURL = maliciousURL + errorData = error + loadSpecialErrorPage(url: maliciousURL) + return true case .navigationNotHandled: return false } @@ -88,7 +91,6 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { failedURL = sslSpecialError.error.url sslErrorPageNavigationHandler.errorPageVisited(errorType: sslSpecialError.type) errorData = sslSpecialError.error.errorData - errorPageType = .ssl loadSpecialErrorPage(url: sslSpecialError.error.url) } @@ -105,45 +107,52 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { extension SpecialErrorPageNavigationHandler: SpecialErrorPageUserScriptDelegate { + @MainActor func leaveSiteAction() { - switch errorPageType { + defer { + if webView?.canGoBack == true { + _ = webView?.goBack() + } else { + delegate?.closeSpecialErrorPageTab() + } + } + + guard let errorData else { return } + + switch errorData { case .ssl: sslErrorPageNavigationHandler.leaveSite() - case .phishing: + case .maliciousSite: maliciousSiteProtectionNavigationHandler.leaveSite() - default: - break - } - - if webView?.canGoBack == true { - _ = webView?.goBack() - } else { - delegate?.closeSpecialErrorPageTab() } } + @MainActor func visitSiteAction() { - switch errorPageType { - case .ssl: - sslErrorPageNavigationHandler.visitSite() - case .phishing: - maliciousSiteProtectionNavigationHandler.visitSite() - default: - break + defer { + isSpecialErrorPageVisible = false + _ = webView?.reload() } - isSpecialErrorPageVisible = false - _ = webView?.reload() + guard let errorData, let url = webView?.url else { return } + + switch errorData { + case .ssl: + sslErrorPageNavigationHandler.visitSite(url: url, errorData: errorData) + case .maliciousSite: + maliciousSiteProtectionNavigationHandler.visitSite(url: url, errorData: errorData) + } } + @MainActor func advancedInfoPresented() { - switch errorPageType { + guard let errorData else { return } + + switch errorData { case .ssl: sslErrorPageNavigationHandler.advancedInfoPresented() - case .phishing: + case .maliciousSite: maliciousSiteProtectionNavigationHandler.advancedInfoPresented() - default: - break } } } @@ -163,3 +172,25 @@ private extension SpecialErrorPageNavigationHandler { } } + +extension SpecialErrorData { + + var url: URL? { + switch self { + case .ssl: + return nil + case let .maliciousSite(_, url): + return url + } + } + + var threatKind: ThreatKind? { + switch self { + case .ssl: + return nil + case let .maliciousSite(threatKind, _): + return threatKind + } + } + +} diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 4586314235..44c4bce2e7 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1846,19 +1846,28 @@ extension TabViewController: WKNavigationDelegate { } } - decidePolicyFor(navigationAction: navigationAction) { [weak self] decision in - if let self = self, - let url = navigationAction.request.url, - decision != .cancel, - navigationAction.isTargetingMainFrame() { - if url.isDuckDuckGoSearch { - StatisticsLoader.shared.refreshSearchRetentionAtb() - privacyProDataReporter.saveSearchCount() + Task { @MainActor in + // Check if should show a special error page for malicious site + if let url = navigationAction.request.url, + !specialErrorPageNavigationHandler.isSpecialErrorPageVisible, + await specialErrorPageNavigationHandler.handleSpecialErrorNavigation(navigationAction: navigationAction, webView: webView) { + decisionHandler(.cancel) + } else { + decidePolicyFor(navigationAction: navigationAction) { [weak self] decision in + if let self = self, + let url = navigationAction.request.url, + decision != .cancel, + navigationAction.isTargetingMainFrame() { + if url.isDuckDuckGoSearch { + StatisticsLoader.shared.refreshSearchRetentionAtb() + privacyProDataReporter.saveSearchCount() + } + self.delegate?.closeFindInPage(tab: self) + } + decisionHandler(decision) } - - self.delegate?.closeFindInPage(tab: self) } - decisionHandler(decision) + } } // swiftlint:enable cyclomatic_complexity From 4a753bdb05c9d822d4fd6c44a3f1c60edf74d799 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 2 Dec 2024 13:07:33 +0100 Subject: [PATCH 02/15] Integrate MaliciousSiteProtection --- DuckDuckGo.xcodeproj/project.pbxproj | 10 +++++ .../xcshareddata/swiftpm/Package.resolved | 40 ++++++++++++++----- ...rPageNavigationHandler+MaliciousSite.swift | 3 +- ...pecialErrorPageNavigationHandler+SSL.swift | 2 +- .../SpecialErrorPageNavigationHandler.swift | 1 + .../SSLErrorPageNavigationHandlerTests.swift | 5 ++- ...ageNavigationHandlerIntegrationTests.swift | 12 +++--- ...ecialErrorPageNavigationHandlerTests.swift | 5 ++- ...ciousSiteProtectionNavigationHandler.swift | 3 +- .../MockSSLErrorPageNavigationHandler.swift | 2 +- 10 files changed, 59 insertions(+), 24 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index dfc65d37f4..9a52bac856 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -771,6 +771,7 @@ 9F254ADC2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254ADA2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift */; }; 9F254ADE2CF636CF0063B308 /* DummyWKNavigation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254ADD2CF636CF0063B308 /* DummyWKNavigation.swift */; }; 9F254ADF2CF636CF0063B308 /* DummyWKNavigation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254ADD2CF636CF0063B308 /* DummyWKNavigation.swift */; }; + 9F254AF12CF8D5250063B308 /* MaliciousSiteProtection in Frameworks */ = {isa = PBXBuildFile; productRef = 9F254AF02CF8D5250063B308 /* MaliciousSiteProtection */; }; 9F254AFF2CF9FA1B0063B308 /* WebViewNavigationHandling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AFE2CF9FA1B0063B308 /* WebViewNavigationHandling.swift */; }; 9F254B012CF9FA8D0063B308 /* SpecialErrorPageActionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254B002CF9FA8D0063B308 /* SpecialErrorPageActionHandler.swift */; }; 9F254B032CF9FB2E0063B308 /* SpecialErrorPageNavigationDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254B022CF9FB2E0063B308 /* SpecialErrorPageNavigationDelegate.swift */; }; @@ -2678,6 +2679,7 @@ 9FB027182C26BC29009EA190 /* BrowsersComparisonModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowsersComparisonModel.swift; sourceTree = ""; }; 9FB0271A2C2927D0009EA190 /* OnboardingView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingView.swift; sourceTree = ""; }; 9FB0271C2C293619009EA190 /* OnboardingIntroViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingIntroViewModel.swift; sourceTree = ""; }; + 9FBC76652CFDF64B008B21E7 /* BrowserServicesKit */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = BrowserServicesKit; path = ../BrowserServicesKit; sourceTree = SOURCE_ROOT; }; 9FCFCD7D2C6AF52A006EB7A0 /* LaunchOptionsHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = LaunchOptionsHandler.swift; path = ../DuckDuckGo/LaunchOptionsHandler.swift; sourceTree = ""; }; 9FCFCD7F2C6AF56D006EB7A0 /* LaunchOptionsHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LaunchOptionsHandlerTests.swift; sourceTree = ""; }; 9FCFCD842C75C91A006EB7A0 /* ProgressBarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressBarView.swift; sourceTree = ""; }; @@ -3172,6 +3174,7 @@ 31E69A63280F4CB600478327 /* DuckUI in Frameworks */, CB941A6E2B96AB08000F9E7A /* PrivacyDashboard in Frameworks */, F42D541D29DCA40B004C4FF1 /* DesignResourcesKit in Frameworks */, + 9F254AF12CF8D5250063B308 /* MaliciousSiteProtection in Frameworks */, 1E5918472CA422A7008ED2B3 /* Navigation in Frameworks */, 85875B6129912A9900115F05 /* SyncUI in Frameworks */, F4D7F634298C00C3006C3AE9 /* FindInPageIOSJSSupport in Frameworks */, @@ -4343,6 +4346,7 @@ 84E341891E2F7EFB00BDBA6F = { isa = PBXGroup; children = ( + 9FBC76652CFDF64B008B21E7 /* BrowserServicesKit */, EE3B98EB2A963515002F63A0 /* WidgetsExtensionAlpha.entitlements */, 6FB030C7234331B400A10DB9 /* Configuration.xcconfig */, EEB8FDB92A990AEE00EBEDCF /* Configuration-Alpha.xcconfig */, @@ -6896,6 +6900,7 @@ 9F96F73A2C9144D5009E45D5 /* Onboarding */, 1E5918462CA422A7008ED2B3 /* Navigation */, 315C77812CFA41A400699683 /* AIChat */, + 9F254AF02CF8D5250063B308 /* MaliciousSiteProtection */, ); productName = DuckDuckGo; productReference = 84E341921E2F7EFB00BDBA6F /* DuckDuckGo.app */; @@ -8599,6 +8604,7 @@ 9F254ADE2CF636CF0063B308 /* DummyWKNavigation.swift in Sources */, 8551912724746EDC0010FDD0 /* SnapshotHelper.swift in Sources */, 9F254ADB2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift in Sources */, + 9F254ACF2CF5D3540063B308 /* SpecialErrorPageNavigationHandlerIntegrationTests.swift in Sources */, 9F254AD82CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -11965,6 +11971,10 @@ package = 98A16C2928A11BDE00A6C003 /* XCRemoteSwiftPackageReference "BrowserServicesKit" */; productName = Bookmarks; }; + 9F254AF02CF8D5250063B308 /* MaliciousSiteProtection */ = { + isa = XCSwiftPackageProductDependency; + productName = MaliciousSiteProtection; + }; 9F8FE9482BAE50E50071E372 /* Lottie */ = { isa = XCSwiftPackageProductDependency; package = 9F8FE9472BAE50E50071E372 /* XCRemoteSwiftPackageReference "lottie-spm" */; diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index e696bd0530..3b80e94535 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -27,15 +27,6 @@ "version" : "3.0.0" } }, - { - "identity" : "browserserviceskit", - "kind" : "remoteSourceControl", - "location" : "https://github.com/DuckDuckGo/BrowserServicesKit", - "state" : { - "revision" : "9975e63265e617ce9c25ae1be6d531f6de5e6592", - "version" : "221.0.0" - } - }, { "identity" : "content-scope-scripts", "kind" : "remoteSourceControl", @@ -59,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/duckduckgo-autofill.git", "state" : { - "revision" : "88982a3802ac504e2f1a118a73bfdf2d8f4a7735", - "version" : "16.0.0" + "revision" : "c992041d16ec10d790e6204dce9abf9966d1363c", + "version" : "15.1.0" } }, { @@ -144,6 +135,24 @@ "version" : "1.4.0" } }, + { + "identity" : "swift-clocks", + "kind" : "remoteSourceControl", + "location" : "https://github.com/pointfreeco/swift-clocks.git", + "state" : { + "revision" : "b9b24b69e2adda099a1fa381cda1eeec272d5b53", + "version" : "1.0.5" + } + }, + { + "identity" : "swift-concurrency-extras", + "kind" : "remoteSourceControl", + "location" : "https://github.com/pointfreeco/swift-concurrency-extras", + "state" : { + "revision" : "163409ef7dae9d960b87f34b51587b6609a76c1f", + "version" : "1.3.0" + } + }, { "identity" : "swift-syntax", "kind" : "remoteSourceControl", @@ -198,6 +207,15 @@ "version" : "1.1.3" } }, + { + "identity" : "xctest-dynamic-overlay", + "kind" : "remoteSourceControl", + "location" : "https://github.com/pointfreeco/xctest-dynamic-overlay", + "state" : { + "revision" : "a3f634d1a409c7979cabc0a71b3f26ffa9fc8af1", + "version" : "1.4.3" + } + }, { "identity" : "zipfoundation", "kind" : "remoteSourceControl", diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index 448cb61f3f..0143be51ab 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -22,6 +22,7 @@ import BrowserServicesKit import Core import SpecialErrorPages import WebKit +import MaliciousSiteProtection enum MaliciousSiteProtectionNavigationResult: Equatable { case navigationHandled(NavigationType) @@ -46,7 +47,7 @@ protocol MaliciousSiteProtectionNavigationHandling: AnyObject { final class MaliciousSiteProtectionNavigationHandler { private let maliciousSiteProtectionManager: MaliciousSiteDetecting private let storageCache: StorageCache - private var maliciousURLExemptions: [URL: MaliciousSiteProtection.ThreatKind] = [:] + private var maliciousURLExemptions: [URL: ThreatKind] = [:] private var bypassedMaliciousSiteThreatKind: ThreatKind? init( diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift index 10d5b71f3a..2f7a062047 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift @@ -90,7 +90,7 @@ extension SSLErrorPageNavigationHandler: SpecialErrorPageActionHandler { Pixel.fire(pixel: .certificateWarningLeaveClicked) } - func visitSite() { + func visitSite(url: URL, errorData: SpecialErrorData) { shouldBypassSSLError = true } diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index cb750ae495..042ff99fe7 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -21,6 +21,7 @@ import Foundation import WebKit import SpecialErrorPages import Core +import MaliciousSiteProtection typealias SpecialErrorPageManaging = SpecialErrorPageContextHandling & WebViewNavigationHandling & SpecialErrorPageUserScriptDelegate diff --git a/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift index d7b7ce923b..73aff959e0 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift @@ -132,12 +132,13 @@ final class SSLSpecialErrorPageTests { } @Test - func whenDidReceiveChallengeIfChallengeForCertificateValidationAndUserRequestBypassThenReturnsCredentials() { + func whenDidReceiveChallengeIfChallengeForCertificateValidationAndUserRequestBypassThenReturnsCredentials() throws { // GIVEN let protectionSpace = URLProtectionSpace(host: "", port: 4, protocol: nil, realm: nil, authenticationMethod: NSURLAuthenticationMethodServerTrust) let challenge = URLAuthenticationChallenge(protectionSpace: protectionSpace, proposedCredential: nil, previousFailureCount: 0, failureResponse: nil, error: nil, sender: ChallengeSender()) var expectedCredential: URLCredential? - sut.visitSite() + let dummyURL = try #require(URL(string: "https://example.com")) + sut.visitSite(url: dummyURL, errorData: .ssl(type: .invalid, domain: "", eTldPlus1: nil)) // WHEN sut.handleServerTrustChallenge(challenge) { _, credential in diff --git a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift index 415386e733..905ddb5184 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift @@ -50,7 +50,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { @Test func whenCertificateExpiredThenExpectedErrorPageIsShown() async throws { // GIVEN - let error = NSError(domain: "test", + let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: ["_kCFStreamErrorCodeKey": errSSLCertExpired, NSURLErrorFailingURLErrorKey: try #require(URL(string: "https://expired.badssl.com"))]) @@ -84,7 +84,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { @Test func whenCertificateWrongHostThenExpectedErrorPageIsShown() async throws { // GIVEN - let error = NSError(domain: "test", + let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: ["_kCFStreamErrorCodeKey": errSSLHostNameMismatch, NSURLErrorFailingURLErrorKey: try #require(URL(string: "https://wrong.host.badssl.com"))]) @@ -118,7 +118,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { @Test func whenCertificateSelfSignedThenExpectedErrorPageIsShown() async throws { // GIVEN - let error = NSError(domain: "test", + let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: ["_kCFStreamErrorCodeKey": errSSLXCertChainInvalid, NSURLErrorFailingURLErrorKey: try #require(URL(string: "https://self-signed.badssl.com"))]) @@ -152,7 +152,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { @Test func whenOtherCertificateIssueThenExpectedErrorPageIsShown() async throws { // GIVEN - let error = NSError(domain: "test", + let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: ["_kCFStreamErrorCodeKey": errSSLUnknownRootCert, NSURLErrorFailingURLErrorKey: try #require(URL(string: "https://untrusted-root.badssl.com"))]) @@ -203,7 +203,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { @Test func whenNavigationEndedIfSSLFailureButURLIsDifferentFromNavigationURLThenSSLUserScriptIsNotEnabled() throws { // GIVEN - let error = NSError(domain: "test", + let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: ["_kCFStreamErrorCodeKey": errSSLUnknownRootCert, NSURLErrorFailingURLErrorKey: try #require(URL(string: "https://untrusted-root.badssl.com"))]) @@ -234,7 +234,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { sut.attachWebView(webView) let navigation = DummyWKNavigation() let error = NSError( - domain: "test", + domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: [ "_kCFStreamErrorCodeKey": errSSLCertExpired, diff --git a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift index d5ea9bd078..4916aa3b33 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift @@ -113,8 +113,11 @@ final class SpecialErrorPageNavigationHandlerTests { @MainActor @Test("Visit Site forward event to SSL Error Page Navigation Handler") - func whenVisitSite_AndSSLError_ThenCallVisitSiteOnSSLErrorPageNavigationHandler() { + func whenVisitSite_AndSSLError_ThenCallVisitSiteOnSSLErrorPageNavigationHandler() throws { // GIVEN + let url = try #require(URL(string: "https://example.com")) + webView.setCurrentURL(url) + sut.attachWebView(webView) sut.handleWebView(webView, didFailProvisionalNavigation: DummyWKNavigation(), withError: .genericSSL) #expect(!sslErrorPageNavigationHandler.didCallVisitSite) diff --git a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift index e842c63cfa..6442cce2af 100644 --- a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift +++ b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift @@ -19,6 +19,7 @@ import Foundation import WebKit +import SpecialErrorPages @testable import DuckDuckGo class DummyMaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling & SpecialErrorPageActionHandler { @@ -26,7 +27,7 @@ class DummyMaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavi .navigationNotHandled } - func visitSite() {} + func visitSite(url: URL, errorData: SpecialErrorData) {} func leaveSite() {} diff --git a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockSSLErrorPageNavigationHandler.swift b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockSSLErrorPageNavigationHandler.swift index 0580bb6f37..7abd659d2a 100644 --- a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockSSLErrorPageNavigationHandler.swift +++ b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockSSLErrorPageNavigationHandler.swift @@ -51,7 +51,7 @@ final class MockSSLErrorPageNavigationHandler: SSLSpecialErrorPageNavigationHand capturedSpecialErrorType = errorType } - func visitSite() { + func visitSite(url: URL, errorData: SpecialErrorData) { didCallVisitSite = true } From c02a209f47caba0113abc5dd06ad1790368ef573 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 2 Dec 2024 13:09:26 +0100 Subject: [PATCH 03/15] Hardcode malicious site detection --- .../MaliciousSiteProtectionManager.swift | 37 ++++++------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift b/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift index 5175a16879..577a95d73c 100644 --- a/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift +++ b/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift @@ -18,39 +18,24 @@ // import Foundation - -final class MaliciousSiteProtectionManager: MaliciousSiteDetecting { - - func evaluate(_ url: URL) async -> ThreatKind? { - try? await Task.sleep(interval: 0.3) - return .none - } - -} - -// MARK: - To Remove - -// These entities are copied from BSK and they will be used to mock the library -import SpecialErrorPages +import MaliciousSiteProtection protocol MaliciousSiteDetecting { func evaluate(_ url: URL) async -> ThreatKind? } -public enum ThreatKind: String, CaseIterable, CustomStringConvertible { - public var description: String { rawValue } - - case phishing - case malware -} +class MaliciousSiteProtectionManager: MaliciousSiteDetecting { -public extension ThreatKind { + func evaluate(_ url: URL) async -> ThreatKind? { + try? await Task.sleep(interval: 0.3) - var errorPageType: SpecialErrorKind { - switch self { - case .malware: .phishing // WIP in BSK - case .phishing: .phishing + switch url.absoluteString { + case "http://privacy-test-pages.site/security/badware/phishing.html": + return .phishing + case "http://privacy-test-pages.site/security/badware/malware.html": + return .malware + default: + return .none } } - } From 3ede0ed825c6a6d07d3e431f49ecd7747a6edb9d Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Tue, 3 Dec 2024 10:13:07 +0100 Subject: [PATCH 04/15] Use isSpecialErrorPageRequest flag to avoid navigation loops --- .../SpecialErrorPageContextHandling.swift | 2 ++ .../SpecialErrorPage/SpecialErrorPageNavigationHandler.swift | 5 +++++ DuckDuckGo/TabViewController.swift | 4 +--- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift index 0c3ce79206..69834864cf 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift @@ -32,6 +32,8 @@ protocol SpecialErrorPageContextHandling: AnyObject { /// The URL that failed to load, if any. var failedURL: URL? { get } + var isSpecialErrorPageRequest: Bool { get } + /// Attaches a web view to the special error page handling. func attachWebView(_ webView: WKWebView) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index 042ff99fe7..fb63559200 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -30,6 +30,7 @@ final class SpecialErrorPageNavigationHandler: SpecialErrorPageContextHandling { private(set) var errorData: SpecialErrorData? private(set) var isSpecialErrorPageVisible = false private(set) var failedURL: URL? + private(set) var isSpecialErrorPageRequest = false private weak var userScript: SpecialErrorPageUserScript? weak var delegate: SpecialErrorPageNavigationDelegate? @@ -66,16 +67,19 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { case let .navigationHandled(.mainFrame(error)): var request = navigationAction.request request.url = error.url + isSpecialErrorPageRequest = true failedURL = error.url errorData = error loadSpecialErrorPage(request: request) return true case let .navigationHandled(.iFrame(maliciousURL, error)): + isSpecialErrorPageRequest = true failedURL = maliciousURL errorData = error loadSpecialErrorPage(url: maliciousURL) return true case .navigationNotHandled: + isSpecialErrorPageRequest = false return false } } @@ -96,6 +100,7 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { } func handleWebView(_ webView: WKWebView, didFinish navigation: WebViewNavigation) { + isSpecialErrorPageRequest = false userScript?.isEnabled = webView.url == failedURL if webView.url != failedURL { isSpecialErrorPageVisible = false diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 44c4bce2e7..698b3626e6 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1848,8 +1848,7 @@ extension TabViewController: WKNavigationDelegate { Task { @MainActor in // Check if should show a special error page for malicious site - if let url = navigationAction.request.url, - !specialErrorPageNavigationHandler.isSpecialErrorPageVisible, + if !specialErrorPageNavigationHandler.isSpecialErrorPageRequest, await specialErrorPageNavigationHandler.handleSpecialErrorNavigation(navigationAction: navigationAction, webView: webView) { decisionHandler(.cancel) } else { @@ -1867,7 +1866,6 @@ extension TabViewController: WKNavigationDelegate { decisionHandler(decision) } } - } } // swiftlint:enable cyclomatic_complexity From 09a196295f2accc0ad3ee092015ca5cbe69d1cf0 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Tue, 3 Dec 2024 10:14:36 +0100 Subject: [PATCH 05/15] Clean up due to integration with BSK --- .../MaliciousSiteProtectionManager.swift | 4 ---- .../SpecialErrorPageNavigationHandler+SSL.swift | 6 +++++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift b/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift index 577a95d73c..321c243f54 100644 --- a/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift +++ b/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift @@ -20,10 +20,6 @@ import Foundation import MaliciousSiteProtection -protocol MaliciousSiteDetecting { - func evaluate(_ url: URL) async -> ThreatKind? -} - class MaliciousSiteProtectionManager: MaliciousSiteDetecting { func evaluate(_ url: URL) async -> ThreatKind? { diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift index 2f7a062047..67a7efc087 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+SSL.swift @@ -71,7 +71,11 @@ extension SSLErrorPageNavigationHandler: SSLSpecialErrorPageNavigationHandling { return nil } - let errorData = SpecialErrorData.ssl(type: errorType, domain: host, eTldPlus1: storageCache.tld.eTLDplus1(host)) + let errorData = SpecialErrorData.ssl( + type: errorType, + domain: host, + eTldPlus1: storageCache.tld.eTLDplus1(host) + ) return SSLSpecialError(type: errorType, error: SpecialErrorModel(url: failedURL, errorData: errorData)) } From 77a9db10f86bac333e43f28f3d6bc127932be5fa Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Tue, 3 Dec 2024 10:15:48 +0100 Subject: [PATCH 06/15] Add test for malicious site navigation handler --- DuckDuckGo.xcodeproj/project.pbxproj | 11 +- ...rPageNavigationHandler+MaliciousSite.swift | 6 +- ...SiteProtectionNavigationHandlerTests.swift | 173 ++++++++++++++++++ .../MockMaliciousSiteProtectionManager.swift | 32 ++++ 4 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift create mode 100644 DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionManager.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 9a52bac856..19a83d7429 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -822,6 +822,9 @@ 9FB027192C26BC29009EA190 /* BrowsersComparisonModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FB027182C26BC29009EA190 /* BrowsersComparisonModel.swift */; }; 9FB0271B2C2927D0009EA190 /* OnboardingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FB0271A2C2927D0009EA190 /* OnboardingView.swift */; }; 9FB0271D2C293619009EA190 /* OnboardingIntroViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FB0271C2C293619009EA190 /* OnboardingIntroViewModel.swift */; }; + 9FBC76672CFE33B5008B21E7 /* MaliciousSiteProtectionNavigationHandlerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FBC76662CFE33B5008B21E7 /* MaliciousSiteProtectionNavigationHandlerTests.swift */; }; + 9FBC766A2CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FBC76692CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift */; }; + 9FBC766B2CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FBC76692CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift */; }; 9FCFCD802C6AF56D006EB7A0 /* LaunchOptionsHandlerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FCFCD7F2C6AF56D006EB7A0 /* LaunchOptionsHandlerTests.swift */; }; 9FCFCD812C6B020D006EB7A0 /* LaunchOptionsHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FCFCD7D2C6AF52A006EB7A0 /* LaunchOptionsHandler.swift */; }; 9FCFCD852C75C91A006EB7A0 /* ProgressBarView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FCFCD842C75C91A006EB7A0 /* ProgressBarView.swift */; }; @@ -2680,6 +2683,8 @@ 9FB0271A2C2927D0009EA190 /* OnboardingView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingView.swift; sourceTree = ""; }; 9FB0271C2C293619009EA190 /* OnboardingIntroViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingIntroViewModel.swift; sourceTree = ""; }; 9FBC76652CFDF64B008B21E7 /* BrowserServicesKit */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = BrowserServicesKit; path = ../BrowserServicesKit; sourceTree = SOURCE_ROOT; }; + 9FBC76662CFE33B5008B21E7 /* MaliciousSiteProtectionNavigationHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MaliciousSiteProtectionNavigationHandlerTests.swift; sourceTree = ""; }; + 9FBC76692CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockMaliciousSiteProtectionManager.swift; sourceTree = ""; }; 9FCFCD7D2C6AF52A006EB7A0 /* LaunchOptionsHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = LaunchOptionsHandler.swift; path = ../DuckDuckGo/LaunchOptionsHandler.swift; sourceTree = ""; }; 9FCFCD7F2C6AF56D006EB7A0 /* LaunchOptionsHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LaunchOptionsHandlerTests.swift; sourceTree = ""; }; 9FCFCD842C75C91A006EB7A0 /* ProgressBarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressBarView.swift; sourceTree = ""; }; @@ -5041,6 +5046,7 @@ CBC88EE22C7F8B1700F0F8C5 /* SSLErrorPageNavigationHandlerTests.swift */, 9F254ACA2CF5CDC60063B308 /* SpecialErrorPageNavigationHandlerTests.swift */, 9F254ACD2CF5D3540063B308 /* SpecialErrorPageNavigationHandlerIntegrationTests.swift */, + 9FBC76662CFE33B5008B21E7 /* MaliciousSiteProtectionNavigationHandlerTests.swift */, ); path = SpecialErrorPage; sourceTree = ""; @@ -5053,6 +5059,7 @@ 9F254AD72CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift */, 9F254ADA2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift */, 9F254ADD2CF636CF0063B308 /* DummyWKNavigation.swift */, + 9FBC76692CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift */, ); path = TestDoubles; sourceTree = ""; @@ -8424,6 +8431,7 @@ 9F254ADF2CF636CF0063B308 /* DummyWKNavigation.swift in Sources */, 6F7BACD42CEE084B00F561D8 /* OmniBarEqualityCheckTests.swift in Sources */, 6F7FB8E72C66197E00867DA7 /* NewTabPageSectionsSettingsModelTests.swift in Sources */, + 9FBC766A2CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift in Sources */, 851CD674244D7E6000331B98 /* UserDefaultsExtension.swift in Sources */, 569437362BE5160600C0881B /* SyncSettingsViewControllerErrorTests.swift in Sources */, EEC02C162B065BE00045CE11 /* NetworkProtectionVPNLocationViewModelTests.swift in Sources */, @@ -8497,6 +8505,7 @@ 9F4CC51F2C48D758006A96EB /* ContextualDaxDialogsFactoryTests.swift in Sources */, 8521FDE6238D414B00A44CC3 /* FileStoreTests.swift in Sources */, F14E491F1E391CE900DC037C /* URLExtensionTests.swift in Sources */, + 9FBC76672CFE33B5008B21E7 /* MaliciousSiteProtectionNavigationHandlerTests.swift in Sources */, 9F23B8062C2BE22700950875 /* OnboardingIntroViewModelTests.swift in Sources */, 9F69331D2C5A191400CD6A5D /* MockTutorialSettings.swift in Sources */, 85D2187424BF25CD004373D2 /* FaviconsTests.swift in Sources */, @@ -8600,11 +8609,11 @@ files = ( 9F254AD62CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift in Sources */, 85F21DB0210F5E32002631A6 /* AtbIntegrationTests.swift in Sources */, + 9FBC766B2CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift in Sources */, 9F254AD22CF5D3A80063B308 /* MockSpecialErrorWebView.swift in Sources */, 9F254ADE2CF636CF0063B308 /* DummyWKNavigation.swift in Sources */, 8551912724746EDC0010FDD0 /* SnapshotHelper.swift in Sources */, 9F254ADB2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift in Sources */, - 9F254ACF2CF5D3540063B308 /* SpecialErrorPageNavigationHandlerIntegrationTests.swift in Sources */, 9F254AD82CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index 0143be51ab..3c0e7927cf 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -47,9 +47,9 @@ protocol MaliciousSiteProtectionNavigationHandling: AnyObject { final class MaliciousSiteProtectionNavigationHandler { private let maliciousSiteProtectionManager: MaliciousSiteDetecting private let storageCache: StorageCache - private var maliciousURLExemptions: [URL: ThreatKind] = [:] - private var bypassedMaliciousSiteThreatKind: ThreatKind? - + private(set) var maliciousURLExemptions: [URL: ThreatKind] = [:] + private(set) var bypassedMaliciousSiteThreatKind: ThreatKind? + init( maliciousSiteProtectionManager: MaliciousSiteDetecting = MaliciousSiteProtectionManager(), storageCache: StorageCache = AppDependencyProvider.shared.storageCache diff --git a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift new file mode 100644 index 0000000000..646eee8a5a --- /dev/null +++ b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift @@ -0,0 +1,173 @@ +// +// MaliciousSiteProtectionNavigationHandlerTests.swift +// DuckDuckGo +// +// 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 Testing +import WebKit +import SpecialErrorPages +import MaliciousSiteProtection +@testable import DuckDuckGo + +@Suite("Special Error Pages - Malicious Site Protection Navigation Handler Unit Tests", .serialized) +struct MaliciousSiteProtectionNavigationHandlerTests { + private var sut: MaliciousSiteProtectionNavigationHandler! + private var mockMaliciousSiteProtectionManager: MockMaliciousSiteProtectionManager! + private var webView: MockWebView! + + @MainActor + init() { + webView = MockWebView() + mockMaliciousSiteProtectionManager = MockMaliciousSiteProtectionManager() + sut = MaliciousSiteProtectionNavigationHandler(maliciousSiteProtectionManager: mockMaliciousSiteProtectionManager) + } + + @Test( + "URLs that should not be handled", + arguments: [ + "about:blank", + "https://duckduckgo.com?q=swift-testing", + "duck://player" + ] + ) + func unhandledURLTypes(path: String) async throws { + // GIVEN + let url = try #require(URL(string: path)) + let navigationAction = await MockNavigationAction(request: URLRequest(url: url)) + + // WHEN + let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + + // THEN + #expect(result == .navigationNotHandled) + } + + @Test("Non Bypassed Malicious Site should not allow navigation", arguments: [ThreatKind.phishing, .malware]) + func whenBypassedMaliciousSiteThreatKindIsNotSetThenReturnNavigationHandled(threat: ThreatKind) async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + mockMaliciousSiteProtectionManager.threatKind = threat + let navigationAction = await MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + + // WHEN + let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + + // THEN + #expect(result == .navigationHandled(.mainFrame(.maliciousSite(kind: threat, url: url)))) + } + + @Test("Bypassed Malicious Site should allow navigation", arguments: [ThreatKind.phishing, .malware]) + func whenBypassedMaliciousSiteThreatKindIsSetThenReturnNavigationNotHandled(threat: ThreatKind) async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + mockMaliciousSiteProtectionManager.threatKind = threat + sut.visitSite(url: url, errorData: .maliciousSite(kind: threat, url: url)) + let navigationAction = await MockNavigationAction(request: URLRequest(url: url)) + + // WHEN + let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + + // THEN + #expect(result == .navigationNotHandled) + } + + @Test("Do not handle navigation when Threat is nil") + func whenThreatKindIsNilThenReturnNavigationNotHandled() async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationAction = await MockNavigationAction(request: URLRequest(url: url)) + mockMaliciousSiteProtectionManager.threatKind = nil + + // WHEN + let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + + // THEN + #expect(result == .navigationNotHandled) + } + + @Test( + "Handle knows threats Main Frame", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenThreatKindIsNotNil_AndNavigationIsMainFrame_ThenReturnNavigationHandledMainFrame(threat: ThreatKind) async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationAction = await MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + mockMaliciousSiteProtectionManager.threatKind = threat + + // WHEN + let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + + // THEN + #expect(result == .navigationHandled(.mainFrame(.maliciousSite(kind: threat, url: url)))) + } + + @Test( + "Handle knows threats IFrame", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenThreatKindIsNotNil_AndNavigationIsIFrame_ThenReturnNavigationHandledIFrame(threat: ThreatKind) async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationAction = await MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: false)) + mockMaliciousSiteProtectionManager.threatKind = threat + + // WHEN + let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + + // THEN + #expect(result == .navigationHandled(.iFrame(maliciousURL: url, error: .maliciousSite(kind: threat, url: url)))) + } + + @Test( + "Visit Site sets Exemption URL and Threat Kind", + arguments: [ + ThreatKind.phishing, .malware + ] + ) + func whenVisitSiteActionThenSetExemptionURLAndThreatKind(threat: ThreatKind) throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: url) + #expect(sut.maliciousURLExemptions.isEmpty) + #expect(sut.bypassedMaliciousSiteThreatKind == nil) + + // WHEN + sut.visitSite(url: url, errorData: errorData) + + // THEN + #expect(sut.maliciousURLExemptions[url] == threat) + #expect(sut.bypassedMaliciousSiteThreatKind == threat) + } + + @Test("Leave Site Pixel", .disabled("Will be implmented in upcoming PR")) + func whenLeaveSiteActionThenFirePixel() throws { + + } + + @Test("Advanced Site Info Pixel", .disabled("Will be implmented in upcoming PR")) + func whenAdvancedSiteInfoActionThenFirePixel() throws { + + } + +} diff --git a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionManager.swift b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionManager.swift new file mode 100644 index 0000000000..325bb5a46d --- /dev/null +++ b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionManager.swift @@ -0,0 +1,32 @@ +// +// MockMaliciousSiteProtectionManager.swift +// DuckDuckGo +// +// 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 MaliciousSiteProtection +@testable import DuckDuckGo + +final class MockMaliciousSiteProtectionManager: MaliciousSiteDetecting { + + var threatKind: ThreatKind? + + func evaluate(_ url: URL) async -> MaliciousSiteProtection.ThreatKind? { + threatKind + } + +} From bf9930e5e46d84d37aa62d97d09471ee8de54927 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Tue, 3 Dec 2024 10:17:24 +0100 Subject: [PATCH 07/15] Disable adding redirect to examption URLs --- .../SpecialErrorPageNavigationHandler+MaliciousSite.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index 3c0e7927cf..f252867115 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -74,7 +74,7 @@ extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavig return .navigationNotHandled } - handleMaliciousExemptions(for: navigationAction.navigationType, url: url) + //handleMaliciousExemptions(for: navigationAction, url: url) guard !shouldBypassMaliciousSiteProtection(for: url) else { return .navigationNotHandled From f1ba746da04d884e14687137bec54feb97c8a494 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Wed, 4 Dec 2024 09:52:16 +0100 Subject: [PATCH 08/15] Re-set bypassed threat kind at every page load --- ...SpecialErrorPageNavigationHandler+MaliciousSite.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index f252867115..790a8dd2d5 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -74,7 +74,7 @@ extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavig return .navigationNotHandled } - //handleMaliciousExemptions(for: navigationAction, url: url) + handleMaliciousExemptions(for: navigationAction.navigationType, url: url) guard !shouldBypassMaliciousSiteProtection(for: url) else { return .navigationNotHandled @@ -123,9 +123,10 @@ extension MaliciousSiteProtectionNavigationHandler: SpecialErrorPageActionHandle private extension MaliciousSiteProtectionNavigationHandler { func handleMaliciousExemptions(for navigationType: WKNavigationType, url: URL) { - if let threatKind = bypassedMaliciousSiteThreatKind, navigationType == .other { - maliciousURLExemptions[url] = threatKind - } +// if let threatKind = bypassedMaliciousSiteThreatKind, navigationType == .other { +// maliciousURLExemptions[url] = threatKind +// } + // Re-set the flag every time we load a web page bypassedMaliciousSiteThreatKind = maliciousURLExemptions[url] } From af5428ac8d10b8b47bea9851220829f798818274 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 9 Dec 2024 17:33:43 +0100 Subject: [PATCH 09/15] Refactor MaliciousSiteProtectionNavigationHandler --- .../Model/SpecialErrorModel.swift | 6 ++ ...rPageNavigationHandler+MaliciousSite.swift | 98 ++++++++++++++----- ...SiteProtectionNavigationHandlerTests.swift | 91 ++++++++++++----- 3 files changed, 145 insertions(+), 50 deletions(-) diff --git a/DuckDuckGo/SpecialErrorPage/Model/SpecialErrorModel.swift b/DuckDuckGo/SpecialErrorPage/Model/SpecialErrorModel.swift index cd4b3b3411..8e1f344fcb 100644 --- a/DuckDuckGo/SpecialErrorPage/Model/SpecialErrorModel.swift +++ b/DuckDuckGo/SpecialErrorPage/Model/SpecialErrorModel.swift @@ -19,6 +19,7 @@ import Foundation import SpecialErrorPages +import WebKit struct SpecialErrorModel: Equatable { let url: URL @@ -29,3 +30,8 @@ struct SSLSpecialError { let type: SSLErrorType let error: SpecialErrorModel } + +struct MaliciousSiteDetectionNavigationResponse: Equatable { + let navigationAction: WKNavigationAction + let errorData: SpecialErrorData +} diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index 790a8dd2d5..bf69afc402 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -29,26 +29,41 @@ enum MaliciousSiteProtectionNavigationResult: Equatable { case navigationNotHandled enum NavigationType: Equatable { - case mainFrame(SpecialErrorData) + case mainFrame(MaliciousSiteDetectionNavigationResponse) case iFrame(maliciousURL: URL, error: SpecialErrorData) } } protocol MaliciousSiteProtectionNavigationHandling: AnyObject { - /// Decides whether to cancel navigation to prevent opening the YouTube app from the web view. + /// Creates a task for detecting malicious sites based on the provided navigation action. /// /// - Parameters: - /// - navigationAction: The navigation action to evaluate. - /// - webView: The web view where navigation is occurring. - /// - Returns: `true` if the navigation should be canceled, `false` otherwise. - func handleMaliciousSiteProtectionNavigation(for navigationAction: WKNavigationAction, webView: WKWebView) async -> MaliciousSiteProtectionNavigationResult + /// - navigationAction: The `WKNavigationAction` object that contains information about + /// the navigation event. + /// - webView: The web view from which the navigation request began. + @MainActor + func creatMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) + + /// Retrieves a task for detecting malicious sites based on the provided navigation response. + /// + /// - Parameters: + /// - navigationResponse: The `WKNavigationResponse` object that contains information about + /// the navigation event. + /// - webView: The web view from which the navigation request began. + /// - Returns: A `Task?` that represents the + /// asynchronous operation for detecting malicious sites. If the task cannot be found, + /// the function returns `nil`. + @MainActor + func getMaliciousSiteDectionTask(for navigationResponse: WKNavigationResponse, webView: WKWebView) -> Task? } final class MaliciousSiteProtectionNavigationHandler { private let maliciousSiteProtectionManager: MaliciousSiteDetecting private let storageCache: StorageCache - private(set) var maliciousURLExemptions: [URL: ThreatKind] = [:] - private(set) var bypassedMaliciousSiteThreatKind: ThreatKind? + + @MainActor private(set) var maliciousURLExemptions: [URL: ThreatKind] = [:] + @MainActor private(set) var bypassedMaliciousSiteThreatKind: ThreatKind? + @MainActor private(set) var maliciousSiteDetectionTasks: [URL: Task] = [:] init( maliciousSiteProtectionManager: MaliciousSiteDetecting = MaliciousSiteProtectionManager(), @@ -64,35 +79,53 @@ final class MaliciousSiteProtectionNavigationHandler { extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling { @MainActor - func handleMaliciousSiteProtectionNavigation(for navigationAction: WKNavigationAction, webView: WKWebView) async -> MaliciousSiteProtectionNavigationResult { + func creatMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) { guard let url = navigationAction.request.url else { - return .navigationNotHandled + return } if let aboutBlankURL = URL(string: "about:blank"), url == aboutBlankURL { - return .navigationNotHandled + return } handleMaliciousExemptions(for: navigationAction.navigationType, url: url) guard !shouldBypassMaliciousSiteProtection(for: url) else { - return .navigationNotHandled + return } - guard let threatKind = await maliciousSiteProtectionManager.evaluate(url) else { - return .navigationNotHandled + let threatDetectionTask: Task = Task.detached { [weak self] in + guard let self else { return .navigationNotHandled } + + guard let threatKind = await self.maliciousSiteProtectionManager.evaluate(url) else { + return .navigationNotHandled + } + + if await navigationAction.isTargetingMainFrame { + let errorData = SpecialErrorData.maliciousSite(kind: threatKind, url: url) + let response = MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: errorData) + return .navigationHandled(.mainFrame(response)) + } else { + // Extract the URL of the source frame (the iframe) that initiated the navigation action + let iFrameTopURL = await navigationAction.sourceFrame.safeRequest?.url ?? url + let errorData = SpecialErrorData.maliciousSite(kind: threatKind, url: iFrameTopURL) + return .navigationHandled(.iFrame(maliciousURL: url, error: errorData)) + } } - if navigationAction.isTargetingMainFrame { - let errorData = SpecialErrorData.maliciousSite(kind: threatKind, url: url) - return .navigationHandled(.mainFrame(errorData)) - } else { - // Extract the URL of the source frame (the iframe) that initiated the navigation action - let iFrameTopURL = navigationAction.sourceFrame.safeRequest?.url ?? url - let errorData = SpecialErrorData.maliciousSite(kind: threatKind, url: iFrameTopURL) - return .navigationHandled(.iFrame(maliciousURL: url, error: errorData)) + maliciousSiteDetectionTasks[url] = threatDetectionTask + } + + @MainActor + func getMaliciousSiteDectionTask(for navigationResponse: WKNavigationResponse, webView: WKWebView) -> Task? { + + guard let url = navigationResponse.response.url else { + assertionFailure("Could not find Malicious Site Detection Task for URL") + return nil } + + return maliciousSiteDetectionTasks.removeValue(forKey: url) } } @@ -122,16 +155,31 @@ extension MaliciousSiteProtectionNavigationHandler: SpecialErrorPageActionHandle private extension MaliciousSiteProtectionNavigationHandler { + @MainActor func handleMaliciousExemptions(for navigationType: WKNavigationType, url: URL) { -// if let threatKind = bypassedMaliciousSiteThreatKind, navigationType == .other { -// maliciousURLExemptions[url] = threatKind -// } + // TODO: check storing redirects // Re-set the flag every time we load a web page bypassedMaliciousSiteThreatKind = maliciousURLExemptions[url] } + @MainActor func shouldBypassMaliciousSiteProtection(for url: URL) -> Bool { bypassedMaliciousSiteThreatKind != .none || url.isDuckDuckGo || url.isDuckURLScheme } } + +// MARK: - Helpers + +private extension SpecialErrorData { + + var threatKind: ThreatKind? { + switch self { + case .ssl: + return nil + case let .maliciousSite(threatKind, _): + return threatKind + } + } + +} diff --git a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift index 646eee8a5a..94350563aa 100644 --- a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift @@ -36,8 +36,9 @@ struct MaliciousSiteProtectionNavigationHandlerTests { sut = MaliciousSiteProtectionNavigationHandler(maliciousSiteProtectionManager: mockMaliciousSiteProtectionManager) } + @MainActor @Test( - "URLs that should not be handled", + "URLs that should not be handled do not create a Malicious Detection Task", arguments: [ "about:blank", "https://duckduckgo.com?q=swift-testing", @@ -47,60 +48,83 @@ struct MaliciousSiteProtectionNavigationHandlerTests { func unhandledURLTypes(path: String) async throws { // GIVEN let url = try #require(URL(string: path)) - let navigationAction = await MockNavigationAction(request: URLRequest(url: url)) + let navigationAction = MockNavigationAction(request: URLRequest(url: url)) // WHEN - let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) // THEN - #expect(result == .navigationNotHandled) + #expect(sut.maliciousSiteDetectionTasks[url] == nil) } - @Test("Non Bypassed Malicious Site should not allow navigation", arguments: [ThreatKind.phishing, .malware]) - func whenBypassedMaliciousSiteThreatKindIsNotSetThenReturnNavigationHandled(threat: ThreatKind) async throws { + @MainActor + @Test("Non Bypassed Malicious Site creates a Malicious Detection Task", arguments: [ThreatKind.phishing, .malware]) + func whenBypassedMaliciousSiteThreatKindIsNotSetThenReturnNavigationHandled(threat: ThreatKind) throws { // GIVEN let url = try #require(URL(string: "https://www.example.com")) mockMaliciousSiteProtectionManager.threatKind = threat - let navigationAction = await MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) // WHEN - let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) // THEN - #expect(result == .navigationHandled(.mainFrame(.maliciousSite(kind: threat, url: url)))) + #expect(sut.maliciousSiteDetectionTasks[url] != nil) } - @Test("Bypassed Malicious Site should allow navigation", arguments: [ThreatKind.phishing, .malware]) - func whenBypassedMaliciousSiteThreatKindIsSetThenReturnNavigationNotHandled(threat: ThreatKind) async throws { + @MainActor + @Test("Bypassed Malicious Site does not create a Malicious Detection Task", arguments: [ThreatKind.phishing, .malware]) + func whenBypassedMaliciousSiteThreatKindIsSetThenReturnNavigationNotHandled(threat: ThreatKind) throws { // GIVEN let url = try #require(URL(string: "https://www.example.com")) mockMaliciousSiteProtectionManager.threatKind = threat sut.visitSite(url: url, errorData: .maliciousSite(kind: threat, url: url)) - let navigationAction = await MockNavigationAction(request: URLRequest(url: url)) + let navigationAction = MockNavigationAction(request: URLRequest(url: url)) // WHEN - let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) // THEN - #expect(result == .navigationNotHandled) + #expect(sut.maliciousSiteDetectionTasks[url] == nil) } + @MainActor + @Test("Retrieving Malicious Site Detection Task Nullifies it") + func whenHandleDecidePolicyForNavigationResponse_AndTaskIsNotNil_ReturnTaskAndRemoveItFromTheDictionary() throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationAction = MockNavigationAction(request: URLRequest(url: url)) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + let navigationResponse = MockNavigationResponse.with(url: url) + #expect(sut.maliciousSiteDetectionTasks[url] != nil) + + // WHEN + _ = try #require(sut.getMaliciousSiteDectionTask(for: navigationResponse, webView: webView)) + + // THEN + #expect(sut.maliciousSiteDetectionTasks[url] == nil) + } + + @MainActor @Test("Do not handle navigation when Threat is nil") func whenThreatKindIsNilThenReturnNavigationNotHandled() async throws { // GIVEN let url = try #require(URL(string: "https://www.example.com")) - let navigationAction = await MockNavigationAction(request: URLRequest(url: url)) + let navigationAction = MockNavigationAction(request: URLRequest(url: url)) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + let navigationResponse = MockNavigationResponse.with(url: url) mockMaliciousSiteProtectionManager.threatKind = nil // WHEN - let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + let result = try #require(sut.getMaliciousSiteDectionTask(for: navigationResponse, webView: webView)) // THEN - #expect(result == .navigationNotHandled) + #expect(await result.value == .navigationNotHandled) } + @MainActor @Test( - "Handle knows threats Main Frame", + "Handle known threat in Main Frame", arguments: [ ThreatKind.phishing, .malware @@ -109,18 +133,21 @@ struct MaliciousSiteProtectionNavigationHandlerTests { func whenThreatKindIsNotNil_AndNavigationIsMainFrame_ThenReturnNavigationHandledMainFrame(threat: ThreatKind) async throws { // GIVEN let url = try #require(URL(string: "https://www.example.com")) - let navigationAction = await MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + let navigationResponse = MockNavigationResponse.with(url: url) mockMaliciousSiteProtectionManager.threatKind = threat // WHEN - let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + let result = try #require(sut.getMaliciousSiteDectionTask(for: navigationResponse, webView: webView)) // THEN - #expect(result == .navigationHandled(.mainFrame(.maliciousSite(kind: threat, url: url)))) + #expect(await result.value == .navigationHandled(.mainFrame(MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: .maliciousSite(kind: threat, url: url))))) } + @MainActor @Test( - "Handle knows threats IFrame", + "Handle known threat in IFrame", arguments: [ ThreatKind.phishing, .malware @@ -129,16 +156,19 @@ struct MaliciousSiteProtectionNavigationHandlerTests { func whenThreatKindIsNotNil_AndNavigationIsIFrame_ThenReturnNavigationHandledIFrame(threat: ThreatKind) async throws { // GIVEN let url = try #require(URL(string: "https://www.example.com")) - let navigationAction = await MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: false)) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: false)) + sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + let navigationResponse = MockNavigationResponse.with(url: url) mockMaliciousSiteProtectionManager.threatKind = threat // WHEN - let result = await sut.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) + let result = try #require(sut.getMaliciousSiteDectionTask(for: navigationResponse, webView: webView)) // THEN - #expect(result == .navigationHandled(.iFrame(maliciousURL: url, error: .maliciousSite(kind: threat, url: url)))) + #expect(await result.value == .navigationHandled(.iFrame(maliciousURL: url, error: .maliciousSite(kind: threat, url: url)))) } + @MainActor @Test( "Visit Site sets Exemption URL and Threat Kind", arguments: [ @@ -171,3 +201,14 @@ struct MaliciousSiteProtectionNavigationHandlerTests { } } + +extension MockNavigationResponse { + + static func with(url: URL) -> MockNavigationResponse { + let response = MockNavigationResponse() + response.url = url + response.mimeType = "text/html" + return response + } + +} From 9543384167b441a43f3b5068f124f7205ac0b7d9 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 9 Dec 2024 17:40:18 +0100 Subject: [PATCH 10/15] Refactor SpecialErrorPageNavigationHandler and handle synchronisation --- DuckDuckGo.xcodeproj/project.pbxproj | 12 +- .../SpecialErrorPageActionHandler.swift | 3 + .../WebViewNavigationHandling.swift | 16 +- .../SpecialErrorPageNavigationHandler.swift | 87 ++++--- DuckDuckGoTests/DownloadMocks.swift | 3 +- .../SSLErrorPageNavigationHandlerTests.swift | 1 + ...ageNavigationHandlerIntegrationTests.swift | 3 +- ...ecialErrorPageNavigationHandlerTests.swift | 242 +++++++++++++++++- ...ciousSiteProtectionNavigationHandler.swift | 35 --- ...ciousSiteProtectionNavigationHandler.swift | 70 +++++ 10 files changed, 375 insertions(+), 97 deletions(-) delete mode 100644 DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift create mode 100644 DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 19a83d7429..63c3e80307 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -763,8 +763,8 @@ 9F254ACE2CF5D3540063B308 /* SpecialErrorPageNavigationHandlerIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254ACD2CF5D3540063B308 /* SpecialErrorPageNavigationHandlerIntegrationTests.swift */; }; 9F254AD22CF5D3A80063B308 /* MockSpecialErrorWebView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD12CF5D3A20063B308 /* MockSpecialErrorWebView.swift */; }; 9F254AD32CF5D3A80063B308 /* MockSpecialErrorWebView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD12CF5D3A20063B308 /* MockSpecialErrorWebView.swift */; }; - 9F254AD52CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD42CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift */; }; - 9F254AD62CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD42CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift */; }; + 9F254AD52CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD42CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift */; }; + 9F254AD62CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD42CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift */; }; 9F254AD82CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD72CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift */; }; 9F254AD92CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254AD72CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift */; }; 9F254ADB2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254ADA2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift */; }; @@ -2630,7 +2630,7 @@ 9F254ACA2CF5CDC60063B308 /* SpecialErrorPageNavigationHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpecialErrorPageNavigationHandlerTests.swift; sourceTree = ""; }; 9F254ACD2CF5D3540063B308 /* SpecialErrorPageNavigationHandlerIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpecialErrorPageNavigationHandlerIntegrationTests.swift; sourceTree = ""; }; 9F254AD12CF5D3A20063B308 /* MockSpecialErrorWebView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSpecialErrorWebView.swift; sourceTree = ""; }; - 9F254AD42CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DummyMaliciousSiteProtectionNavigationHandler.swift; sourceTree = ""; }; + 9F254AD42CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockMaliciousSiteProtectionNavigationHandler.swift; sourceTree = ""; }; 9F254AD72CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSSLErrorPageNavigationHandler.swift; sourceTree = ""; }; 9F254ADA2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSpecialErrorPageNavigationDelegate.swift; sourceTree = ""; }; 9F254ADD2CF636CF0063B308 /* DummyWKNavigation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DummyWKNavigation.swift; sourceTree = ""; }; @@ -5055,7 +5055,7 @@ isa = PBXGroup; children = ( 9F254AD12CF5D3A20063B308 /* MockSpecialErrorWebView.swift */, - 9F254AD42CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift */, + 9F254AD42CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift */, 9F254AD72CF605310063B308 /* MockSSLErrorPageNavigationHandler.swift */, 9F254ADA2CF6120E0063B308 /* MockSpecialErrorPageNavigationDelegate.swift */, 9F254ADD2CF636CF0063B308 /* DummyWKNavigation.swift */, @@ -8391,7 +8391,7 @@ 569437342BE4E41500C0881B /* SyncErrorHandlerSyncErrorsAlertsTests.swift in Sources */, CBC88EE32C7F8B1700F0F8C5 /* SSLErrorPageNavigationHandlerTests.swift in Sources */, 85C11E4120904BBE00BFFEB4 /* VariantManagerTests.swift in Sources */, - 9F254AD52CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift in Sources */, + 9F254AD52CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift in Sources */, F1134ECE1F40EA9C00B73467 /* AtbParserTests.swift in Sources */, F189AEE41F18FDAF001EBAE1 /* LinkTests.swift in Sources */, 6F7FB8E12C660B3E00867DA7 /* NewTabPageFavoritesModelTests.swift in Sources */, @@ -8607,7 +8607,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 9F254AD62CF5E5B10063B308 /* DummyMaliciousSiteProtectionNavigationHandler.swift in Sources */, + 9F254AD62CF5E5B10063B308 /* MockMaliciousSiteProtectionNavigationHandler.swift in Sources */, 85F21DB0210F5E32002631A6 /* AtbIntegrationTests.swift in Sources */, 9FBC766B2CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift in Sources */, 9F254AD22CF5D3A80063B308 /* MockSpecialErrorWebView.swift in Sources */, diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift index d939be97cf..6315a30c30 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageActionHandler.swift @@ -27,11 +27,14 @@ import SpecialErrorPages /// advanced information related to the error. protocol SpecialErrorPageActionHandler { /// Handles the action of navigating to the site associated with the error page + @MainActor func visitSite(url: URL, errorData: SpecialErrorData) /// Handles the action of leaving the site associated with the error page + @MainActor func leaveSite() /// Handles the action of requesting more detailed information about the error + @MainActor func advancedInfoPresented() } diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/WebViewNavigationHandling.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/WebViewNavigationHandling.swift index 31cb18a9bf..74dc85f68b 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/WebViewNavigationHandling.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/WebViewNavigationHandling.swift @@ -19,6 +19,7 @@ import Foundation import WebKit +import MaliciousSiteProtection // MARK: - WebViewNavigation @@ -41,8 +42,16 @@ protocol WebViewNavigationHandling: AnyObject { /// - Parameters: /// - navigationAction: Details about the action that triggered the navigation request. /// - webView: The web view from which the navigation request began. - /// - Returns: A Boolean value that indicates whether the navigation action was handled. - func handleSpecialErrorNavigation(navigationAction: WKNavigationAction, webView: WKWebView) async -> Bool + @MainActor + func handleDecidePolicyFor(navigationAction: WKNavigationAction, webView: WKWebView) + + /// Decides whether to to navigate to new content after the response to the navigation request is known or cancel the navigation and show a special error page based on the specified action information. + /// - Parameters: + /// - navigationResponse: Descriptive information about the navigation response. + /// - webView: The web view from which the navigation request began. + /// - Returns: A Boolean value that indicates whether to cancel or allow the navigation. + @MainActor + func handleDecidePolicyfor(navigationResponse: WKNavigationResponse, webView: WKWebView) async -> Bool /// Handles authentication challenges received by the web view. /// @@ -50,6 +59,7 @@ protocol WebViewNavigationHandling: AnyObject { /// - webView: The web view that receives the authentication challenge. /// - challenge: The authentication challenge. /// - completionHandler: A completion handler block to execute with the response. + @MainActor func handleWebView(_ webView: WKWebView, didReceive challenge: URLAuthenticationChallenge, completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) /// Handles failures during provisional navigation. @@ -58,6 +68,7 @@ protocol WebViewNavigationHandling: AnyObject { /// - webView: The `WKWebView` instance that failed the navigation. /// - navigation: The navigation object for the operation. /// - error: The error that occurred. + @MainActor func handleWebView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WebViewNavigation, withError error: NSError) /// Handles the successful completion of a navigation in the web view. @@ -65,5 +76,6 @@ protocol WebViewNavigationHandling: AnyObject { /// - Parameters: /// - webView: The web view that loaded the content. /// - navigation: The navigation object that finished. + @MainActor func handleWebView(_ webView: WKWebView, didFinish navigation: WebViewNavigation) } diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index fb63559200..c034e5b184 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -27,13 +27,14 @@ typealias SpecialErrorPageManaging = SpecialErrorPageContextHandling & WebViewNa final class SpecialErrorPageNavigationHandler: SpecialErrorPageContextHandling { private var webView: WKWebView? - private(set) var errorData: SpecialErrorData? - private(set) var isSpecialErrorPageVisible = false - private(set) var failedURL: URL? - private(set) var isSpecialErrorPageRequest = false private weak var userScript: SpecialErrorPageUserScript? weak var delegate: SpecialErrorPageNavigationDelegate? + @MainActor private(set) var errorData: SpecialErrorData? + @MainActor private(set) var isSpecialErrorPageVisible = false + @MainActor private(set) var failedURL: URL? + @MainActor private(set) var isSpecialErrorPageRequest = false + private let sslErrorPageNavigationHandler: SSLSpecialErrorPageNavigationHandling & SpecialErrorPageActionHandler private let maliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling & SpecialErrorPageActionHandler @@ -59,38 +60,48 @@ final class SpecialErrorPageNavigationHandler: SpecialErrorPageContextHandling { extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { - func handleSpecialErrorNavigation(navigationAction: WKNavigationAction, webView: WKWebView) async -> Bool { - let result = await maliciousSiteProtectionNavigationHandler.handleMaliciousSiteProtectionNavigation(for: navigationAction, webView: webView) - - return await MainActor.run { - switch result { - case let .navigationHandled(.mainFrame(error)): - var request = navigationAction.request - request.url = error.url - isSpecialErrorPageRequest = true - failedURL = error.url - errorData = error - loadSpecialErrorPage(request: request) - return true - case let .navigationHandled(.iFrame(maliciousURL, error)): - isSpecialErrorPageRequest = true - failedURL = maliciousURL - errorData = error - loadSpecialErrorPage(url: maliciousURL) - return true - case .navigationNotHandled: - isSpecialErrorPageRequest = false - return false - } + @MainActor + func handleDecidePolicyFor(navigationAction: WKNavigationAction, webView: WKWebView) { + maliciousSiteProtectionNavigationHandler.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + } + + @MainActor + func handleDecidePolicyfor(navigationResponse: WKNavigationResponse, webView: WKWebView) async -> Bool { + guard let task = maliciousSiteProtectionNavigationHandler.getMaliciousSiteDectionTask(for: navigationResponse, webView: webView) else { + return false + } + + let result = await task.value + + switch result { + case let .navigationHandled(.mainFrame(response)): + var request = response.navigationAction.request + request.url = response.errorData.url + isSpecialErrorPageRequest = true + failedURL = response.errorData.url + errorData = response.errorData + loadSpecialErrorPage(request: request) + return true + case let .navigationHandled(.iFrame(maliciousURL, error)): + isSpecialErrorPageRequest = true + failedURL = maliciousURL + errorData = error + loadSpecialErrorPage(url: maliciousURL) + return true + case .navigationNotHandled: + isSpecialErrorPageRequest = false + return false } } - + + @MainActor func handleWebView(_ webView: WKWebView, didReceive challenge: URLAuthenticationChallenge, completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) { guard challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust else { return } sslErrorPageNavigationHandler.handleServerTrustChallenge(challenge, completionHandler: completionHandler) } - + + @MainActor func handleWebView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WebViewNavigation, withError error: NSError) { guard let sslSpecialError = sslErrorPageNavigationHandler.makeNewRequestURLAndSpecialErrorDataIfEnabled(error: error) else { return } failedURL = sslSpecialError.error.url @@ -98,7 +109,8 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { errorData = sslSpecialError.error.errorData loadSpecialErrorPage(url: sslSpecialError.error.url) } - + + @MainActor func handleWebView(_ webView: WKWebView, didFinish navigation: WebViewNavigation) { isSpecialErrorPageRequest = false userScript?.isEnabled = webView.url == failedURL @@ -167,10 +179,12 @@ extension SpecialErrorPageNavigationHandler: SpecialErrorPageUserScriptDelegate private extension SpecialErrorPageNavigationHandler { + @MainActor func loadSpecialErrorPage(url: URL) { loadSpecialErrorPage(request: URLRequest(url: url)) } + @MainActor func loadSpecialErrorPage(request: URLRequest) { let html = SpecialErrorPageHTMLTemplate.htmlFromTemplate webView?.loadSimulatedRequest(request, responseHTML: html) @@ -179,7 +193,9 @@ private extension SpecialErrorPageNavigationHandler { } -extension SpecialErrorData { +// MARK: - Helpers + +private extension SpecialErrorData { var url: URL? { switch self { @@ -190,13 +206,4 @@ extension SpecialErrorData { } } - var threatKind: ThreatKind? { - switch self { - case .ssl: - return nil - case let .maliciousSite(threatKind, _): - return threatKind - } - } - } diff --git a/DuckDuckGoTests/DownloadMocks.swift b/DuckDuckGoTests/DownloadMocks.swift index 1601392430..9b7267b6e3 100644 --- a/DuckDuckGoTests/DownloadMocks.swift +++ b/DuckDuckGoTests/DownloadMocks.swift @@ -51,11 +51,12 @@ class MockDownloadSession: DownloadSession { } class MockNavigationResponse: WKNavigationResponse { + var url = URL(string: "https://www.duck.com")! var suggestedFileName: String? var mimeType: String? override var response: URLResponse { - let response = MockURLResponse(url: URL(string: "https://www.duck.com")!, + let response = MockURLResponse(url: url, mimeType: mimeType!, expectedContentLength: 1234, textEncodingName: "") diff --git a/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift index 73aff959e0..5ca5c32ef1 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SSLErrorPageNavigationHandlerTests.swift @@ -131,6 +131,7 @@ final class SSLSpecialErrorPageTests { #expect(expectedCredential == nil) } + @MainActor @Test func whenDidReceiveChallengeIfChallengeForCertificateValidationAndUserRequestBypassThenReturnsCredentials() throws { // GIVEN diff --git a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift index 905ddb5184..f655b398bb 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerIntegrationTests.swift @@ -36,7 +36,7 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { sslErrorPageNavigationHandler = SSLErrorPageNavigationHandler(featureFlagger: featureFlagger) sut = SpecialErrorPageNavigationHandler( sslErrorPageNavigationHandler: sslErrorPageNavigationHandler, - maliciousSiteProtectionNavigationHandler: DummyMaliciousSiteProtectionNavigationHandler() + maliciousSiteProtectionNavigationHandler: MockMaliciousSiteProtectionNavigationHandler() ) } @@ -249,5 +249,4 @@ final class SpecialErrorPageNavigationHandlerIntegrationTests { // THEN #expect(script.isEnabled) } - } diff --git a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift index 4916aa3b33..de27627311 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift @@ -20,6 +20,7 @@ import Testing import WebKit import SpecialErrorPages +import MaliciousSiteProtection @testable import DuckDuckGo @Suite("Special Error Pages - SpecialErrorPageNavigationHandler Unit Tests", .serialized) @@ -27,6 +28,7 @@ final class SpecialErrorPageNavigationHandlerTests { private var sut: SpecialErrorPageNavigationHandler! private var webView: MockSpecialErrorWebView! private var sslErrorPageNavigationHandler: MockSSLErrorPageNavigationHandler! + private var maliciousSiteProtectionNavigationHandler: MockMaliciousSiteProtectionNavigationHandler! @MainActor init() { @@ -34,9 +36,10 @@ final class SpecialErrorPageNavigationHandlerTests { featureFlagger.enabledFeatureFlags = [.sslCertificatesBypass] webView = MockSpecialErrorWebView(frame: CGRect(), configuration: .nonPersistent()) sslErrorPageNavigationHandler = MockSSLErrorPageNavigationHandler() + maliciousSiteProtectionNavigationHandler = MockMaliciousSiteProtectionNavigationHandler() sut = SpecialErrorPageNavigationHandler( sslErrorPageNavigationHandler: sslErrorPageNavigationHandler, - maliciousSiteProtectionNavigationHandler: DummyMaliciousSiteProtectionNavigationHandler() + maliciousSiteProtectionNavigationHandler: maliciousSiteProtectionNavigationHandler ) } @@ -46,7 +49,154 @@ final class SpecialErrorPageNavigationHandlerTests { webView = nil } - @Test("Receive Challenge forward event to SSL Error Page Navigation Handler") + @MainActor + @Test("Decide Policy For Navigation Action forwards event to Malicious Site Protection Handler") + func whenHandleDecidePolicyForNavigationActionIsCalledThenAskMaliciousSiteProtectionNavigationHandlerToHandleTheDecision() throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationAction = MockNavigationAction(request: URLRequest(url: url)) + + // WHEN + sut.handleDecidePolicyFor(navigationAction: navigationAction, webView: webView) + + // THEN + #expect(maliciousSiteProtectionNavigationHandler.didCallHandleMaliciousSiteProtectionForNavigationAction) + #expect(maliciousSiteProtectionNavigationHandler.capturedNavigationAction == navigationAction) + #expect(maliciousSiteProtectionNavigationHandler.capturedWebView == webView) + } + + @MainActor + @Test("Decide Policy For Navigation Response forwards event to Malicious Site Protection Handler") + func whenHandleDecidePolicyforNavigationResponseThenAskMaliciousSiteProtectionNavigationHandlerToHandleTheDecision() async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationResponse = MockNavigationResponse.with(url: url) + + // WHEN + _ = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + // THEN + #expect(maliciousSiteProtectionNavigationHandler.didCallHandleMaliciousSiteProtectionForNavigationResponse) + #expect(maliciousSiteProtectionNavigationHandler.capturedNavigationResponse == navigationResponse) + #expect(maliciousSiteProtectionNavigationHandler.capturedWebView == webView) + } + + @MainActor + @Test("Decide Policy For Navigation Response returns false when malicious site detection Task is not found") + func whenHandleDecidePolicyForNavigationResponse_And_TaskIsNil_ThenReturnFalse() async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let navigationResponse = MockNavigationResponse.with(url: url) + maliciousSiteProtectionNavigationHandler.task = nil + + // WHEN + let result = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + // THEN + #expect(result == false) + } + + @MainActor + @Test( + "When Main Frame Threat Then Load Bundled Response And Return True", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenHandleDecidePolicyForNavigationResponse_AndMainFrameThreat_ThenLoadBundledReponseAndReturnTrue(threat: ThreatKind) async throws { + // GIVEN + sut.attachWebView(webView) + let url = try #require(URL(string: "https://www.example.com")) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationResponse = MockNavigationResponse.with(url: url) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: url) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationHandled(.mainFrame(MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: errorData))) + } + var didCallLoadSimulatedRequest = false + webView.loadRequestHandler = { _, _ in + didCallLoadSimulatedRequest = true + } + + // WHEN + let result = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + // THEN + #expect(sut.isSpecialErrorPageRequest) + #expect(sut.failedURL == url) + #expect(sut.errorData == errorData) + #expect(didCallLoadSimulatedRequest) + #expect(result) + } + + @MainActor + @Test( + "When iFrame Threat Then Load Bundled Response And Return True", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenHandleDecidePolicyForNavigationResponse_AndIFrameThreat_ThenLoadBundledReponseAndReturnTrue(threat: ThreatKind) async throws { + // GIVEN + sut.attachWebView(webView) + let topFrameURL = try #require(URL(string: "https://www.example.com")) + let iFrameURL = try #require(URL(string: "https://www.iframe.example.com")) + let navigationResponse = MockNavigationResponse.with(url: topFrameURL) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: topFrameURL) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationHandled(.iFrame(maliciousURL: iFrameURL, error: errorData)) + } + var didCallLoadSimulatedRequest = false + webView.loadRequestHandler = { _, _ in + didCallLoadSimulatedRequest = true + } + + // WHEN + let result = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + // THEN + #expect(sut.isSpecialErrorPageRequest) + #expect(sut.failedURL == iFrameURL) + #expect(sut.errorData == errorData) + #expect(didCallLoadSimulatedRequest) + #expect(result) + } + + @MainActor + @Test( + "When No Threat Found Then Return False", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenHandleDecidePolicyForNavigationResponse_AndNoFrameThreat_ThenReturnFalse(threat: ThreatKind) async throws { + // GIVEN + sut.attachWebView(webView) + let url = try #require(URL(string: "https://www.example.com")) + let navigationResponse = MockNavigationResponse.with(url: url) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationNotHandled + } + var didCallLoadSimulatedRequest = false + webView.loadRequestHandler = { _, _ in + didCallLoadSimulatedRequest = true + } + + // WHEN + let result = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + // THEN + #expect(sut.isSpecialErrorPageRequest == false) + #expect(sut.failedURL == nil) + #expect(didCallLoadSimulatedRequest == false) + #expect(result == false) + } + + @MainActor + @Test("Receive Challenge forwards event to SSL Error Page Navigation Handler") func whenDidHandleWebViewReceiveChallengeIsCalledAskSSLErrorPageNavigationHandlerToHandleTheChallenge() { // GIVEN let protectionSpace = URLProtectionSpace(host: "", port: 4, protocol: nil, realm: nil, authenticationMethod: NSURLAuthenticationMethodServerTrust) @@ -74,13 +224,36 @@ final class SpecialErrorPageNavigationHandlerTests { #expect(sslErrorPageNavigationHandler.didCallLeaveSite) } - @Test("Leave Site forward event to Malicious Site Protection Navigation Handler", .disabled("Will implement in upcoming PR")) - func whenLeaveSite_AndPhishingError_ThenCallLeaveSiteOnMaliciousSiteProtectioneNavigationHandler() { + @MainActor + @Test( + "Leave Site forward event to Malicious Site Protection Navigation Handler", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenLeaveSite_AndMaliciousSiteError_ThenCallLeaveSiteOnMaliciousSiteProtectioneNavigationHandler(threat: ThreatKind) async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: url) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationResponse = MockNavigationResponse.with(url: url) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationHandled(.mainFrame(MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: errorData))) + } + _ = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + #expect(!maliciousSiteProtectionNavigationHandler.didCallLeaveSite) + + // WHEN + sut.leaveSiteAction() + // THEN + #expect(maliciousSiteProtectionNavigationHandler.didCallLeaveSite) } @MainActor - @Test("Lave Site navigate Back") + @Test("Lave Site navigates Back") func whenLeaveSite_AndWebViewCanNavigateBack_ThenNavigateBack() { // GIVEN webView.setCanGoBack(true) @@ -95,7 +268,7 @@ final class SpecialErrorPageNavigationHandlerTests { } @MainActor - @Test("Lave Site close Tab") + @Test("Lave Site closes Tab") func whenLeaveSite_AndWebViewCannotNavigateBack_ThenAskDelegateToCloseTab() { // GIVEN webView.setCanGoBack(false) @@ -128,9 +301,34 @@ final class SpecialErrorPageNavigationHandlerTests { #expect(sslErrorPageNavigationHandler.didCallVisitSite) } - @Test("Visit Site forward event to Malicious Site Protection Navigation Handler", .disabled("Will implement in upcoming PR")) - func whenVisitSite_AndPhishingError_ThenCallVisitSiteOnMaliciousSiteProtectioneNavigationHandler() { + @MainActor + @Test( + "Visit Site forward event to Malicious Site Protection Navigation Handler", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenVisitSite_AndPhishingError_ThenCallVisitSiteOnMaliciousSiteProtectioneNavigationHandler(threat: ThreatKind) async throws { + // GIVEN + let url = try #require(URL(string: "https://www.example.com")) + webView.setCurrentURL(url) + sut.attachWebView(webView) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: url) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationResponse = MockNavigationResponse.with(url: url) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationHandled(.mainFrame(MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: errorData))) + } + _ = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + #expect(!maliciousSiteProtectionNavigationHandler.didCallVisitSite) + + // WHEN + sut.visitSiteAction() + + // THEN + #expect(maliciousSiteProtectionNavigationHandler.didCallVisitSite) } @MainActor @@ -164,16 +362,38 @@ final class SpecialErrorPageNavigationHandlerTests { #expect(sslErrorPageNavigationHandler.didCalladvancedInfoPresented) } - @Test("Advanced Info Presented forward event to Malicious Site Protection Navigation Handler", .disabled("Will implement in upcoming PR")) - func whenAdvancedInfoPresented_AndPhishingError_ThenCallAdvancedInfoPresentedOnMaliciousSiteProtectionNavigationHandler() { + @MainActor + @Test( + "Advanced Info Presented forward event to Malicious Site Protection Navigation Handler", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenAdvancedInfoPresented_AndPhishingError_ThenCallAdvancedInfoPresentedOnMaliciousSiteProtectionNavigationHandler(threat: ThreatKind) async throws { + let url = try #require(URL(string: "https://www.example.com")) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: url) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationResponse = MockNavigationResponse.with(url: url) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationHandled(.mainFrame(MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: errorData))) + } + _ = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + + #expect(!maliciousSiteProtectionNavigationHandler.didCallAdvancedInfoPresented) + + // WHEN + sut.advancedInfoPresented() + // THEN + #expect(maliciousSiteProtectionNavigationHandler.didCallAdvancedInfoPresented) } } private extension NSError { static let genericSSL = NSError( - domain: "test", + domain: NSURLErrorDomain, code: NSURLErrorServerCertificateUntrusted, userInfo: [ "_kCFStreamErrorCodeKey": errSSLUnknownRootCert, diff --git a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift deleted file mode 100644 index 6442cce2af..0000000000 --- a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/DummyMaliciousSiteProtectionNavigationHandler.swift +++ /dev/null @@ -1,35 +0,0 @@ -// -// DummyMaliciousSiteProtectionNavigationHandler.swift -// DuckDuckGo -// -// 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 WebKit -import SpecialErrorPages -@testable import DuckDuckGo - -class DummyMaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling & SpecialErrorPageActionHandler { - func handleMaliciousSiteProtectionNavigation(for navigationAction: WKNavigationAction, webView: WKWebView) async -> DuckDuckGo.MaliciousSiteProtectionNavigationResult { - .navigationNotHandled - } - - func visitSite(url: URL, errorData: SpecialErrorData) {} - - func leaveSite() {} - - func advancedInfoPresented() {} -} diff --git a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift new file mode 100644 index 0000000000..c6d4bdb848 --- /dev/null +++ b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift @@ -0,0 +1,70 @@ +// +// MockMaliciousSiteProtectionNavigationHandler.swift +// DuckDuckGo +// +// 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 WebKit +import SpecialErrorPages +@testable import DuckDuckGo + +final class MockMaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling & SpecialErrorPageActionHandler { + private(set) var didCallHandleMaliciousSiteProtectionForNavigationAction = false + private(set) var capturedNavigationAction: WKNavigationAction? + private(set) var capturedWebView: WKWebView? + + private(set) var didCallHandleMaliciousSiteProtectionForNavigationResponse = false + private(set) var capturedNavigationResponse: WKNavigationResponse? + + private(set) var didCallVisitSite = false + private(set) var capturedVisitSiteURL: URL? + private(set) var capturedErrorData: SpecialErrorData? + + private(set) var didCallLeaveSite = false + + private(set) var didCallAdvancedInfoPresented = false + + var task: Task? + + func creatMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) { + didCallHandleMaliciousSiteProtectionForNavigationAction = true + capturedNavigationAction = navigationAction + capturedWebView = webView + } + + func getMaliciousSiteDectionTask(for navigationResponse: WKNavigationResponse, webView: WKWebView) -> Task? { + didCallHandleMaliciousSiteProtectionForNavigationResponse = true + capturedNavigationResponse = navigationResponse + capturedWebView = webView + + return task + } + + func visitSite(url: URL, errorData: SpecialErrorData) { + didCallVisitSite = true + capturedVisitSiteURL = url + capturedErrorData = errorData + } + + func leaveSite() { + didCallLeaveSite = true + } + + func advancedInfoPresented() { + didCallAdvancedInfoPresented = true + } +} From 040565b413544a6b0d07b472f8c85fc3d1a17de9 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 9 Dec 2024 17:58:08 +0100 Subject: [PATCH 11/15] Fix typo for Malicious site Navigation Handler method --- .../SpecialErrorPageContextHandling.swift | 1 + ...lErrorPageNavigationHandler+MaliciousSite.swift | 4 ++-- .../SpecialErrorPageNavigationHandler.swift | 2 +- ...ciousSiteProtectionNavigationHandlerTests.swift | 14 +++++++------- ...kMaliciousSiteProtectionNavigationHandler.swift | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift index 69834864cf..6719cb0876 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageInterfaces/SpecialErrorPageContextHandling.swift @@ -32,6 +32,7 @@ protocol SpecialErrorPageContextHandling: AnyObject { /// The URL that failed to load, if any. var failedURL: URL? { get } + /// A boolean value indicating whether the WebView request requires showing a special error page. var isSpecialErrorPageRequest: Bool { get } /// Attaches a web view to the special error page handling. diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift index bf69afc402..91c14f6695 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler+MaliciousSite.swift @@ -42,7 +42,7 @@ protocol MaliciousSiteProtectionNavigationHandling: AnyObject { /// the navigation event. /// - webView: The web view from which the navigation request began. @MainActor - func creatMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) + func createMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) /// Retrieves a task for detecting malicious sites based on the provided navigation response. /// @@ -79,7 +79,7 @@ final class MaliciousSiteProtectionNavigationHandler { extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling { @MainActor - func creatMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) { + func createMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) { guard let url = navigationAction.request.url else { return diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index c034e5b184..98668ae993 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -62,7 +62,7 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { @MainActor func handleDecidePolicyFor(navigationAction: WKNavigationAction, webView: WKWebView) { - maliciousSiteProtectionNavigationHandler.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + maliciousSiteProtectionNavigationHandler.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) } @MainActor diff --git a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift index 94350563aa..b55b855f43 100644 --- a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift @@ -51,7 +51,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { let navigationAction = MockNavigationAction(request: URLRequest(url: url)) // WHEN - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) // THEN #expect(sut.maliciousSiteDetectionTasks[url] == nil) @@ -66,7 +66,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) // WHEN - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) // THEN #expect(sut.maliciousSiteDetectionTasks[url] != nil) @@ -82,7 +82,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { let navigationAction = MockNavigationAction(request: URLRequest(url: url)) // WHEN - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) // THEN #expect(sut.maliciousSiteDetectionTasks[url] == nil) @@ -94,7 +94,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { // GIVEN let url = try #require(URL(string: "https://www.example.com")) let navigationAction = MockNavigationAction(request: URLRequest(url: url)) - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) let navigationResponse = MockNavigationResponse.with(url: url) #expect(sut.maliciousSiteDetectionTasks[url] != nil) @@ -111,7 +111,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { // GIVEN let url = try #require(URL(string: "https://www.example.com")) let navigationAction = MockNavigationAction(request: URLRequest(url: url)) - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) let navigationResponse = MockNavigationResponse.with(url: url) mockMaliciousSiteProtectionManager.threatKind = nil @@ -134,7 +134,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { // GIVEN let url = try #require(URL(string: "https://www.example.com")) let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) let navigationResponse = MockNavigationResponse.with(url: url) mockMaliciousSiteProtectionManager.threatKind = threat @@ -157,7 +157,7 @@ struct MaliciousSiteProtectionNavigationHandlerTests { // GIVEN let url = try #require(URL(string: "https://www.example.com")) let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: false)) - sut.creatMaliciousSiteDetectionTask(for: navigationAction, webView: webView) + sut.createMaliciousSiteDetectionTask(for: navigationAction, webView: webView) let navigationResponse = MockNavigationResponse.with(url: url) mockMaliciousSiteProtectionManager.threatKind = threat diff --git a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift index c6d4bdb848..6ece26386b 100644 --- a/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift +++ b/DuckDuckGoTests/SpecialErrorPage/TestDoubles/MockMaliciousSiteProtectionNavigationHandler.swift @@ -40,7 +40,7 @@ final class MockMaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectio var task: Task? - func creatMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) { + func createMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) { didCallHandleMaliciousSiteProtectionForNavigationAction = true capturedNavigationAction = navigationAction capturedWebView = webView From fe4aa8265f5954e15f17fe656d7aa22f1173f652 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 9 Dec 2024 18:16:31 +0100 Subject: [PATCH 12/15] Update the logic for handling malicious site protection --- DuckDuckGo/TabViewController.swift | 131 +++++++++++++++-------------- 1 file changed, 69 insertions(+), 62 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 698b3626e6..252b000f50 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1375,55 +1375,64 @@ extension TabViewController: WKNavigationDelegate { NotificationCenter.default.post(Notification(name: AppUserDefaults.Notifications.didVerifyInternalUser)) } - // Important: Order of these checks matter! - if urlSchemeType == .blob { - // 1. To properly handle BLOB we need to trigger its download, if temporaryDownloadForPreviewedFile is set we allow its load in the web view - if let temporaryDownloadForPreviewedFile, temporaryDownloadForPreviewedFile.url == navigationResponse.response.url { - // BLOB already has a temporary downloaded so and we can allow loading it - blobDownloadTargetFrame = nil - decisionHandler(.allow) - } else { - // First we need to trigger download to handle it then in webView:navigationAction:didBecomeDownload - decisionHandler(.download) - } - } else if FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { - // 2. For this MIME type we are able to provide a better custom preview via FilePreviewHelper so it takes priority - let download = self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) - mostRecentAutoPreviewDownloadID = download?.id - Pixel.fire(pixel: .downloadStarted, - withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) - } else if shouldTriggerDownloadAction(for: navigationResponse), - let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { - // 3a. We know it is a download, but allow WebKit handle the "data" scheme natively - if urlNavigationalScheme == .data { - decisionHandler(.download) - return - } - - // 3b. We know the response should trigger the file download prompt - self.presentSaveToDownloadsAlert(with: downloadMetadata) { - self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) - } cancelHandler: { + // If the navigation has been handled by the special error page handler cancel navigating to new content as the special error page will be shown. + Task { @MainActor in + if + !specialErrorPageNavigationHandler.isSpecialErrorPageRequest, + await specialErrorPageNavigationHandler.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) { decisionHandler(.cancel) - } - } else if navigationResponse.canShowMIMEType { - // 4. WebView can preview the MIME type and it is not to be handled by our custom FilePreviewHelper - url = webView.url - if navigationResponse.isForMainFrame, let decision = setupOrClearTemporaryDownload(for: navigationResponse.response) { - // Loading a file preview in web view - decisionHandler(decision) } else { - // Loading HTML - if navigationResponse.isForMainFrame && isSuccessfulResponse { - adClickAttributionDetection.on2XXResponse(url: url) - } - adClickAttributionLogic.onProvisionalNavigation { + // Important: Order of these checks matter! + if urlSchemeType == .blob { + // 1. To properly handle BLOB we need to trigger its download, if temporaryDownloadForPreviewedFile is set we allow its load in the web view + if let temporaryDownloadForPreviewedFile, temporaryDownloadForPreviewedFile.url == navigationResponse.response.url { + // BLOB already has a temporary downloaded so and we can allow loading it + blobDownloadTargetFrame = nil + decisionHandler(.allow) + } else { + // First we need to trigger download to handle it then in webView:navigationAction:didBecomeDownload + decisionHandler(.download) + } + } else if FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { + // 2. For this MIME type we are able to provide a better custom preview via FilePreviewHelper so it takes priority + let download = self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) + mostRecentAutoPreviewDownloadID = download?.id + Pixel.fire(pixel: .downloadStarted, + withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) + } else if shouldTriggerDownloadAction(for: navigationResponse), + let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { + // 3a. We know it is a download, but allow WebKit handle the "data" scheme natively + if urlNavigationalScheme == .data { + decisionHandler(.download) + return + } + + // 3b. We know the response should trigger the file download prompt + self.presentSaveToDownloadsAlert(with: downloadMetadata) { + self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) + } cancelHandler: { + decisionHandler(.cancel) + } + } else if navigationResponse.canShowMIMEType { + // 4. WebView can preview the MIME type and it is not to be handled by our custom FilePreviewHelper + url = webView.url + if navigationResponse.isForMainFrame, let decision = setupOrClearTemporaryDownload(for: navigationResponse.response) { + // Loading a file preview in web view + decisionHandler(decision) + } else { + // Loading HTML + if navigationResponse.isForMainFrame && isSuccessfulResponse { + adClickAttributionDetection.on2XXResponse(url: url) + } + adClickAttributionLogic.onProvisionalNavigation { + decisionHandler(.allow) + } + } + } else { + // Fallback decisionHandler(.allow) } } - } else { - // Fallback - decisionHandler(.allow) } } @@ -1846,26 +1855,24 @@ extension TabViewController: WKNavigationDelegate { } } - Task { @MainActor in - // Check if should show a special error page for malicious site - if !specialErrorPageNavigationHandler.isSpecialErrorPageRequest, - await specialErrorPageNavigationHandler.handleSpecialErrorNavigation(navigationAction: navigationAction, webView: webView) { - decisionHandler(.cancel) - } else { - decidePolicyFor(navigationAction: navigationAction) { [weak self] decision in - if let self = self, - let url = navigationAction.request.url, - decision != .cancel, - navigationAction.isTargetingMainFrame() { - if url.isDuckDuckGoSearch { - StatisticsLoader.shared.refreshSearchRetentionAtb() - privacyProDataReporter.saveSearchCount() - } - self.delegate?.closeFindInPage(tab: self) - } - decisionHandler(decision) + decidePolicyFor(navigationAction: navigationAction) { [weak self] decision in + if let self = self, + let url = navigationAction.request.url, + decision != .cancel, + navigationAction.isTargetingMainFrame() { + if url.isDuckDuckGoSearch { + StatisticsLoader.shared.refreshSearchRetentionAtb() + privacyProDataReporter.saveSearchCount() } + + self.delegate?.closeFindInPage(tab: self) + } + // If navigating to the URL is allowed and the URL request is not sideloaded ask the specialErrorPageNavigationHandler forward the event to + // the SpecialErrorPageNavigationHandler. + if let self, decision == .allow, !self.specialErrorPageNavigationHandler.isSpecialErrorPageRequest { + self.specialErrorPageNavigationHandler.handleDecidePolicyFor(navigationAction: navigationAction, webView: webView) } + decisionHandler(decision) } } // swiftlint:enable cyclomatic_complexity From feb36be3074f37853ba83340689d1569204918bf Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 9 Dec 2024 18:41:07 +0100 Subject: [PATCH 13/15] Close Tab when leaving malicious site --- .../SpecialErrorPageNavigationHandler.swift | 11 ++++- ...ecialErrorPageNavigationHandlerTests.swift | 42 +++++++++++++++++-- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index 98668ae993..4b6ff7185b 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -127,21 +127,28 @@ extension SpecialErrorPageNavigationHandler: SpecialErrorPageUserScriptDelegate @MainActor func leaveSiteAction() { - defer { + + func navigateBackIfPossible() { if webView?.canGoBack == true { _ = webView?.goBack() } else { - delegate?.closeSpecialErrorPageTab() + forceCloseTab() } } + func forceCloseTab() { + delegate?.closeSpecialErrorPageTab() + } + guard let errorData else { return } switch errorData { case .ssl: sslErrorPageNavigationHandler.leaveSite() + navigateBackIfPossible() case .maliciousSite: maliciousSiteProtectionNavigationHandler.leaveSite() + forceCloseTab() } } diff --git a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift index de27627311..ccb5edb02c 100644 --- a/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/SpecialErrorPageNavigationHandlerTests.swift @@ -253,11 +253,12 @@ final class SpecialErrorPageNavigationHandlerTests { } @MainActor - @Test("Lave Site navigates Back") - func whenLeaveSite_AndWebViewCanNavigateBack_ThenNavigateBack() { + @Test("Lave Site navigates Back when SSL Error") + func whenLeaveSite_AndSSLError_AndWebViewCanNavigateBack_ThenNavigateBack() { // GIVEN webView.setCanGoBack(true) sut.attachWebView(webView) + sut.handleWebView(webView, didFailProvisionalNavigation: DummyWKNavigation(), withError: .genericSSL) #expect(!webView.didCallGoBack) // WHEN @@ -268,14 +269,47 @@ final class SpecialErrorPageNavigationHandlerTests { } @MainActor - @Test("Lave Site closes Tab") - func whenLeaveSite_AndWebViewCannotNavigateBack_ThenAskDelegateToCloseTab() { + @Test("Lave Site closes Tab when SSL Error") + func whenLeaveSite_AndSSLError_AndWebViewCannotNavigateBack_ThenAskDelegateToCloseTab() { // GIVEN webView.setCanGoBack(false) let delegate = SpySpecialErrorPageNavigationDelegate() sut.delegate = delegate sut.attachWebView(webView) + sut.handleWebView(webView, didFailProvisionalNavigation: DummyWKNavigation(), withError: .genericSSL) + #expect(!delegate.didCallCloseSpecialErrorPageTab) + + // WHEN + sut.leaveSiteAction() + + // THEN + #expect(delegate.didCallCloseSpecialErrorPageTab) + } + + @MainActor + @Test( + "Lave Site closes Tab when Malicious Site Error", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) + func whenLeaveSite_AndMaliciousSiteError_AndWebViewCanNavigateBack_ThenNavigateBack(threat: ThreatKind) async throws { + // GIVEN + webView.setCanGoBack(true) + sut.attachWebView(webView) + let url = try #require(URL(string: "https://example.com")) + let errorData = SpecialErrorData.maliciousSite(kind: threat, url: url) + let navigationAction = MockNavigationAction(request: URLRequest(url: url), targetFrame: MockFrameInfo(isMainFrame: true)) + let navigationResponse = MockNavigationResponse.with(url: url) + maliciousSiteProtectionNavigationHandler.task = Task { + .navigationHandled(.mainFrame(MaliciousSiteDetectionNavigationResponse(navigationAction: navigationAction, errorData: errorData))) + } + _ = await sut.handleDecidePolicyfor(navigationResponse: navigationResponse, webView: webView) + let delegate = SpySpecialErrorPageNavigationDelegate() + sut.delegate = delegate #expect(!delegate.didCallCloseSpecialErrorPageTab) + // WHEN sut.leaveSiteAction() From 160bc30a7da51f15a86a0a2c3dd3ceb1f49c7313 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Mon, 9 Dec 2024 18:47:22 +0100 Subject: [PATCH 14/15] Update BSK version --- DuckDuckGo.xcodeproj/project.pbxproj | 2 - .../xcshareddata/swiftpm/Package.resolved | 40 +++++-------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 63c3e80307..1579b5f540 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2682,7 +2682,6 @@ 9FB027182C26BC29009EA190 /* BrowsersComparisonModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowsersComparisonModel.swift; sourceTree = ""; }; 9FB0271A2C2927D0009EA190 /* OnboardingView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingView.swift; sourceTree = ""; }; 9FB0271C2C293619009EA190 /* OnboardingIntroViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingIntroViewModel.swift; sourceTree = ""; }; - 9FBC76652CFDF64B008B21E7 /* BrowserServicesKit */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = BrowserServicesKit; path = ../BrowserServicesKit; sourceTree = SOURCE_ROOT; }; 9FBC76662CFE33B5008B21E7 /* MaliciousSiteProtectionNavigationHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MaliciousSiteProtectionNavigationHandlerTests.swift; sourceTree = ""; }; 9FBC76692CFE3802008B21E7 /* MockMaliciousSiteProtectionManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockMaliciousSiteProtectionManager.swift; sourceTree = ""; }; 9FCFCD7D2C6AF52A006EB7A0 /* LaunchOptionsHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = LaunchOptionsHandler.swift; path = ../DuckDuckGo/LaunchOptionsHandler.swift; sourceTree = ""; }; @@ -4351,7 +4350,6 @@ 84E341891E2F7EFB00BDBA6F = { isa = PBXGroup; children = ( - 9FBC76652CFDF64B008B21E7 /* BrowserServicesKit */, EE3B98EB2A963515002F63A0 /* WidgetsExtensionAlpha.entitlements */, 6FB030C7234331B400A10DB9 /* Configuration.xcconfig */, EEB8FDB92A990AEE00EBEDCF /* Configuration-Alpha.xcconfig */, diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 3b80e94535..e696bd0530 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -27,6 +27,15 @@ "version" : "3.0.0" } }, + { + "identity" : "browserserviceskit", + "kind" : "remoteSourceControl", + "location" : "https://github.com/DuckDuckGo/BrowserServicesKit", + "state" : { + "revision" : "9975e63265e617ce9c25ae1be6d531f6de5e6592", + "version" : "221.0.0" + } + }, { "identity" : "content-scope-scripts", "kind" : "remoteSourceControl", @@ -50,8 +59,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/duckduckgo-autofill.git", "state" : { - "revision" : "c992041d16ec10d790e6204dce9abf9966d1363c", - "version" : "15.1.0" + "revision" : "88982a3802ac504e2f1a118a73bfdf2d8f4a7735", + "version" : "16.0.0" } }, { @@ -135,24 +144,6 @@ "version" : "1.4.0" } }, - { - "identity" : "swift-clocks", - "kind" : "remoteSourceControl", - "location" : "https://github.com/pointfreeco/swift-clocks.git", - "state" : { - "revision" : "b9b24b69e2adda099a1fa381cda1eeec272d5b53", - "version" : "1.0.5" - } - }, - { - "identity" : "swift-concurrency-extras", - "kind" : "remoteSourceControl", - "location" : "https://github.com/pointfreeco/swift-concurrency-extras", - "state" : { - "revision" : "163409ef7dae9d960b87f34b51587b6609a76c1f", - "version" : "1.3.0" - } - }, { "identity" : "swift-syntax", "kind" : "remoteSourceControl", @@ -207,15 +198,6 @@ "version" : "1.1.3" } }, - { - "identity" : "xctest-dynamic-overlay", - "kind" : "remoteSourceControl", - "location" : "https://github.com/pointfreeco/xctest-dynamic-overlay", - "state" : { - "revision" : "a3f634d1a409c7979cabc0a71b3f26ffa9fc8af1", - "version" : "1.4.3" - } - }, { "identity" : "zipfoundation", "kind" : "remoteSourceControl", From b6dbf5c5df1a11d648abeada8bc28f9d79e70d16 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Tue, 10 Dec 2024 10:28:59 +0100 Subject: [PATCH 15/15] Clean up --- .../MaliciousSiteProtectionManager.swift | 3 ++- .../SpecialErrorPageNavigationHandler.swift | 8 +++++--- .../MaliciousSiteProtectionNavigationHandlerTests.swift | 8 +++++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift b/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift index 321c243f54..46dab041d0 100644 --- a/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift +++ b/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift @@ -20,7 +20,7 @@ import Foundation import MaliciousSiteProtection -class MaliciousSiteProtectionManager: MaliciousSiteDetecting { +final class MaliciousSiteProtectionManager: MaliciousSiteDetecting { func evaluate(_ url: URL) async -> ThreatKind? { try? await Task.sleep(interval: 0.3) @@ -34,4 +34,5 @@ class MaliciousSiteProtectionManager: MaliciousSiteDetecting { return .none } } + } diff --git a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift index 4b6ff7185b..c5d596ec77 100644 --- a/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift +++ b/DuckDuckGo/SpecialErrorPage/SpecialErrorPageNavigationHandler.swift @@ -75,6 +75,8 @@ extension SpecialErrorPageNavigationHandler: WebViewNavigationHandling { switch result { case let .navigationHandled(.mainFrame(response)): + // Re-use the same request to avoid that the new sideload request is intercepted and cancelled + // due to parameters added to the header. var request = response.navigationAction.request request.url = response.errorData.url isSpecialErrorPageRequest = true @@ -132,11 +134,11 @@ extension SpecialErrorPageNavigationHandler: SpecialErrorPageUserScriptDelegate if webView?.canGoBack == true { _ = webView?.goBack() } else { - forceCloseTab() + closeTab() } } - func forceCloseTab() { + func closeTab() { delegate?.closeSpecialErrorPageTab() } @@ -148,7 +150,7 @@ extension SpecialErrorPageNavigationHandler: SpecialErrorPageUserScriptDelegate navigateBackIfPossible() case .maliciousSite: maliciousSiteProtectionNavigationHandler.leaveSite() - forceCloseTab() + closeTab() } } diff --git a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift index b55b855f43..96b5a06854 100644 --- a/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift +++ b/DuckDuckGoTests/SpecialErrorPage/MaliciousSiteProtectionNavigationHandlerTests.swift @@ -73,7 +73,13 @@ struct MaliciousSiteProtectionNavigationHandlerTests { } @MainActor - @Test("Bypassed Malicious Site does not create a Malicious Detection Task", arguments: [ThreatKind.phishing, .malware]) + @Test( + "Bypassed Malicious Site does not create a Malicious Detection Task", + arguments: [ + ThreatKind.phishing, + .malware + ] + ) func whenBypassedMaliciousSiteThreatKindIsSetThenReturnNavigationNotHandled(threat: ThreatKind) throws { // GIVEN let url = try #require(URL(string: "https://www.example.com"))