-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Malicious site protection navigation detection #3707
base: alessandro/malicious-site-protection
Are you sure you want to change the base?
Changes from all commits
1e126cd
3986ec1
0712de4
ecd530f
3d8c5e4
b20db83
0310680
c6aef3d
0b52351
8925306
efb6e54
4de6ac8
c7b2d3d
758a164
829bc38
5c07c05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
// | ||
|
||
import Foundation | ||
import SpecialErrorPages | ||
|
||
/// A type that defines actions for handling special error pages. | ||
/// | ||
|
@@ -26,11 +27,14 @@ 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() | ||
@MainActor | ||
func visitSite(url: URL, errorData: SpecialErrorData) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m a bit on the fence with this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you perhaps create two funcs visitSite, one without params and one with params? Different handlers would be implementing only one function leaving other implementation empty? It's not ideal but the simplest There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. However, if we encounter more cases like this where we’re stretching these protocols, we might need to reconsider whether it’s appropriate to treat all special errors the same way. For example, SSL and malicious site errors already exhibit different behaviors. I’ll leave it up to you, but perhaps we should treat them differently (not under same logic) from the start. We can discuss further in 2025 :) |
||
|
||
/// 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() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
|
||
import Foundation | ||
import WebKit | ||
import MaliciousSiteProtection | ||
|
||
// MARK: - WebViewNavigation | ||
|
||
|
@@ -41,15 +42,24 @@ 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be |
||
|
||
/// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo, "for" is lowercased, but I'd refer to my previous comment ^ |
||
|
||
/// Handles authentication challenges received by the web view. | ||
/// | ||
/// - Parameters: | ||
/// - 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,12 +68,14 @@ 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. | ||
/// | ||
/// - Parameters: | ||
/// - webView: The web view that loaded the content. | ||
/// - navigation: The navigation object that finished. | ||
@MainActor | ||
func handleWebView(_ webView: WKWebView, didFinish navigation: WebViewNavigation) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be http vs https?
also, we should be keeping all urls inside AppURLs