From 0f2b9c9722fa965c414e1303904643e7e8cd5288 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 9 Dec 2024 10:36:07 +0100 Subject: [PATCH 01/12] Intercept requests and add skeleton for malicious site detection --- .../app/browser/BrowserWebViewClientTest.kt | 5 +- .../browser/WebViewRequestInterceptorTest.kt | 37 +++++++ .../UrlExtractingWebViewClientTest.kt | 3 + .../referencetests/DomainsReferenceTest.kt | 3 + .../referencetests/SurrogatesReferenceTest.kt | 3 + .../app/browser/BrowserWebViewClient.kt | 15 ++- .../app/browser/WebViewRequestInterceptor.kt | 11 +++ .../app/browser/di/BrowserModule.kt | 3 + .../UrlExtractingWebViewClient.kt | 3 + .../api/MaliciousSiteProtection.kt | 24 ++++- .../impl/RealMaliciousSiteProtection.kt | 99 +++++++++++++++++++ 11 files changed, 203 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index c96b3fd19478..a796fc873d99 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -79,6 +79,7 @@ import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Off import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Unavailable import com.duckduckgo.history.api.NavigationHistory +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.AmpLinks import com.duckduckgo.subscriptions.api.Subscriptions import com.duckduckgo.user.agent.api.ClientBrandHintProvider @@ -152,6 +153,7 @@ class BrowserWebViewClientTest { private val mockUriLoadedManager: UriLoadedManager = mock() private val mockAndroidBrowserConfigFeature: AndroidBrowserConfigFeature = mock() private val mockAndroidFeaturesHeaderPlugin = AndroidFeaturesHeaderPlugin(mockDuckDuckGoUrlDetector, mockAndroidBrowserConfigFeature, mock()) + private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() @UiThreadTest @Before @@ -189,6 +191,7 @@ class BrowserWebViewClientTest { mockDuckDuckGoUrlDetector, mockUriLoadedManager, mockAndroidFeaturesHeaderPlugin, + mockMaliciousSiteProtection, ) testee.webViewClientListener = listener whenever(webResourceRequest.url).thenReturn(Uri.EMPTY) @@ -334,7 +337,7 @@ class BrowserWebViewClientTest { TestScope().launch { val webResourceRequest = mock() testee.shouldInterceptRequest(webView, webResourceRequest) - verify(requestInterceptor).shouldIntercept(any(), any(), any(), any()) + verify(requestInterceptor).shouldIntercept(any(), any(), any(), any(), any()) } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt index 2753e38780ee..8bccdddac934 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt @@ -46,6 +46,7 @@ import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle import com.duckduckgo.httpsupgrade.api.HttpsUpgrader +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.impl.features.gpc.RealGpc.Companion.GPC_HEADER import com.duckduckgo.request.filterer.api.RequestFilterer @@ -97,6 +98,7 @@ class WebViewRequestInterceptorTest { fakeToggle, fakeUserAllowListRepository, ) + private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() private var webView: WebView = mock() @@ -128,6 +130,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) assertCancelledResponse(response) @@ -140,6 +143,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -153,6 +157,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -167,6 +172,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -181,6 +187,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) verify(mockHttpsUpgrader).upgrade(any()) @@ -196,6 +203,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -209,6 +217,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) verify(mockDuckPlayer).intercept(any(), any(), any()) @@ -222,6 +231,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -235,6 +245,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -248,6 +259,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) assertRequestCanContinueToLoad(response) @@ -260,6 +272,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duckduckgo.com/a/b/c?q=123".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -273,6 +286,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "donttrack.us/a/b/c?q=123".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -286,6 +300,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "spreadprivacy.com/a/b/c?q=123".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -299,6 +314,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duckduckhack.com/a/b/c?q=123".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -312,6 +328,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "privatebrowsingmyths.com/a/b/c?q=123".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -325,6 +342,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duck.co/a/b/c?q=123".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -341,6 +359,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockListener, ) @@ -358,6 +377,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockListener, ) @@ -375,6 +395,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -397,6 +418,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -420,6 +442,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -434,6 +457,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -450,6 +474,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -467,6 +492,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -484,6 +510,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -501,6 +528,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -518,6 +546,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -534,6 +563,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -550,6 +580,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -566,6 +597,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -582,6 +614,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -641,6 +674,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -679,6 +713,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -700,6 +735,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -719,6 +755,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt index 52df9a0b1ecb..b97b21da6bde 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt @@ -26,6 +26,7 @@ import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.cookies.api.CookieManagerProvider +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Before @@ -50,6 +51,7 @@ class UrlExtractingWebViewClientTest { private val thirdPartyCookieManager: ThirdPartyCookieManager = mock() private val urlExtractor: DOMUrlExtractor = mock() private val mockWebView: WebView = mock() + private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() @UiThreadTest @Before @@ -63,6 +65,7 @@ class UrlExtractingWebViewClientTest { TestScope(), coroutinesTestRule.testDispatcherProvider, urlExtractor, + mockMaliciousSiteProtection, ) whenever(cookieManagerProvider.get()).thenReturn(cookieManager) } diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt index 9bc809b2efdf..e9efcba2c2b3 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt @@ -57,6 +57,7 @@ import com.duckduckgo.common.test.FileUtilities import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle import com.duckduckgo.httpsupgrade.api.HttpsUpgrader +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.ContentBlocking import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.api.TrackerAllowlist @@ -119,6 +120,7 @@ class DomainsReferenceTest(private val testCase: TestCase) { ) private val mockGpc: Gpc = mock() private val mockAdClickManager: AdClickManager = mock() + private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() companion object { private val moshi = Moshi.Builder().add(ActionJsonAdapter()).build() @@ -198,6 +200,7 @@ class DomainsReferenceTest(private val testCase: TestCase) { request = mockRequest, documentUri = testCase.siteURL.toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt index 83fb7607d59f..1615945a7efa 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt @@ -55,6 +55,7 @@ import com.duckduckgo.common.test.FileUtilities import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle import com.duckduckgo.httpsupgrade.api.HttpsUpgrader +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.ContentBlocking import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.api.TrackerAllowlist @@ -115,6 +116,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) { private val mockGpc: Gpc = mock() private val mockAdClickManager: AdClickManager = mock() private val mockCloakedCnameDetector: CloakedCnameDetector = mock() + private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() companion object { private val moshi = Moshi.Builder().add(ActionJsonAdapter()).build() @@ -187,6 +189,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) { request = mockRequest, documentUri = testCase.siteURL.toUri(), webView = webView, + maliciousSiteProtection = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index de04c2b54cc9..23b7edb1b6db 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -76,6 +76,7 @@ import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.ENABLED import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On import com.duckduckgo.duckplayer.impl.DUCK_PLAYER_OPEN_IN_YOUTUBE_PATH import com.duckduckgo.history.api.NavigationHistory +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.AmpLinks import com.duckduckgo.subscriptions.api.Subscriptions import com.duckduckgo.user.agent.api.ClientBrandHintProvider @@ -117,6 +118,7 @@ class BrowserWebViewClient @Inject constructor( private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector, private val uriLoadedManager: UriLoadedManager, private val androidFeaturesHeaderPlugin: AndroidFeaturesHeaderPlugin, + private val phishingAndMalwareDetector: MaliciousSiteProtection, ) : WebViewClient() { var webViewClientListener: WebViewClientListener? = null @@ -126,6 +128,10 @@ class BrowserWebViewClient @Inject constructor( private var shouldOpenDuckPlayerInNewTab: Boolean = true + private val onSiteBlockedAsync: () -> Unit = { + // TODO (cbarreiro): Handle site blocked asynchronously + } + init { appCoroutineScope.launch { duckPlayer.observeShouldOpenInNewTab().collect { @@ -158,6 +164,12 @@ class BrowserWebViewClient @Inject constructor( try { Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url") webViewClientListener?.onShouldOverride() + + if (phishingAndMalwareDetector.shouldOverrideUrlLoading(url, webView, isForMainFrame, isRedirect, onSiteBlockedAsync)) { + // TODO (cbarreiro): Handle site blocked synchronously + return true + } + if (isForMainFrame && dosDetector.isUrlGeneratingDos(url)) { webView.loadUrl(ABOUT_BLANK) webViewClientListener?.dosAttackDetected() @@ -407,6 +419,7 @@ class BrowserWebViewClient @Inject constructor( // See https://app.asana.com/0/0/1206159443951489/f (WebView limitations) if (it != ABOUT_BLANK && start == null) { start = currentTimeProvider.elapsedRealtime() + phishingAndMalwareDetector.onPageLoadStarted() } handleMediaPlayback(webView, it) autoconsent.injectAutoconsent(webView, url) @@ -509,7 +522,7 @@ class BrowserWebViewClient @Inject constructor( loginDetector.onEvent(WebNavigationEvent.ShouldInterceptRequest(webView, request)) } Timber.v("Intercepting resource ${request.url} type:${request.method} on page $documentUrl") - requestInterceptor.shouldIntercept(request, webView, documentUrl?.toUri(), webViewClientListener) + requestInterceptor.shouldIntercept(request, webView, documentUrl?.toUri(), phishingAndMalwareDetector, webViewClientListener) } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 5ca57ebb0029..2241c5f72003 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -35,6 +35,7 @@ import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.isHttp import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.httpsupgrade.api.HttpsUpgrader +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.request.filterer.api.RequestFilterer import com.duckduckgo.user.agent.api.UserAgentProvider @@ -48,6 +49,7 @@ interface RequestInterceptor { request: WebResourceRequest, webView: WebView, documentUri: Uri?, + maliciousSiteProtection: MaliciousSiteProtection, webViewClientListener: WebViewClientListener?, ): WebResourceResponse? @@ -92,10 +94,19 @@ class WebViewRequestInterceptor( request: WebResourceRequest, webView: WebView, documentUri: Uri?, + maliciousSiteProtection: MaliciousSiteProtection, webViewClientListener: WebViewClientListener?, ): WebResourceResponse? { val url: Uri? = request.url + val onSiteBlockedAsync: () -> Unit = { + // TODO (cbarreiro): Handle site blocked asynchronously + } + maliciousSiteProtection.shouldIntercept(request, webView, documentUri, onSiteBlockedAsync)?.let { + // TODO (cbarreiro): Handle site blocked synchronously + return it + } + if (requestFilterer.shouldFilterOutRequest(request, documentUri.toString())) return WebResourceResponse(null, null, null) adClickManager.detectAdClick(url?.toString(), request.isForMainFrame) diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index 98876d97a533..39b6e7004be7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -84,6 +84,7 @@ import com.duckduckgo.downloads.impl.FileDownloadCallback import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.experiments.api.VariantManager import com.duckduckgo.httpsupgrade.api.HttpsUpgrader +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.AmpLinks import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.api.TrackingParameters @@ -121,6 +122,7 @@ class BrowserModule { @AppCoroutineScope appCoroutineScope: CoroutineScope, dispatcherProvider: DispatcherProvider, urlExtractor: DOMUrlExtractor, + maliciousSiteProtection: MaliciousSiteProtection, ): UrlExtractingWebViewClient { return UrlExtractingWebViewClient( webViewHttpAuthStore, @@ -131,6 +133,7 @@ class BrowserModule { appCoroutineScope, dispatcherProvider, urlExtractor, + maliciousSiteProtection, ) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt index f8c4e2a509cc..dd41f50332ce 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt @@ -30,6 +30,7 @@ import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.cookies.api.CookieManagerProvider +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import kotlinx.coroutines.* import timber.log.Timber @@ -42,6 +43,7 @@ class UrlExtractingWebViewClient( private val appCoroutineScope: CoroutineScope, private val dispatcherProvider: DispatcherProvider, private val urlExtractor: DOMUrlExtractor, + private val maliciousSiteProtection: MaliciousSiteProtection, ) : WebViewClient() { var urlExtractionListener: UrlExtractionListener? = null @@ -82,6 +84,7 @@ class UrlExtractingWebViewClient( request, webView, documentUrl, + maliciousSiteProtection, null, ) } diff --git a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt index ac25c78c3083..8d2f97cd67dc 100644 --- a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt @@ -16,4 +16,26 @@ package com.duckduckgo.malicioussiteprotection.api -interface MaliciousSiteProtection +import android.net.Uri +import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse +import android.webkit.WebView + +interface MaliciousSiteProtection { + suspend fun shouldIntercept( + request: WebResourceRequest, + webView: WebView, + documentUri: Uri?, + onSiteBlockedAsync: () -> Unit, + ): WebResourceResponse? + + fun onPageLoadStarted() + + fun shouldOverrideUrlLoading( + url: Uri, + webView: WebView, + isForMainFrame: Boolean, + isRedirect: Boolean, + onSiteBlockedAsync: () -> Unit, + ): Boolean +} diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index 7cb11d7ccb00..1adfdbcd1093 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -16,6 +16,11 @@ package com.duckduckgo.malicioussiteprotection.impl +import android.net.Uri +import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse +import android.webkit.WebView +import androidx.core.net.toUri import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.di.IsMainProcess import com.duckduckgo.common.utils.DispatcherProvider @@ -24,10 +29,13 @@ import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding +import java.net.URLDecoder import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.json.JSONObject +import timber.log.Timber @ContributesBinding(AppScope::class, MaliciousSiteProtection::class) @ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) @@ -63,4 +71,95 @@ class RealMaliciousSiteProtection @Inject constructor( } } } + + private val processedUrls = mutableListOf() + + private fun shouldIntercept(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean { + // TODO (cbarreiro): Implement the logic to check if the URL is malicious + return false + } + + override suspend fun shouldIntercept( + request: WebResourceRequest, + webView: WebView, + documentUri: Uri?, + onSiteBlockedAsync: () -> Unit, + ): WebResourceResponse? { + if (!isFeatureEnabled) { + return null + } + val url = request.url.let { + if (it.fragment != null) { + it.buildUpon().fragment(null).build() + } else { + it + } + } + + val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() + + if (processedUrls.contains(decodedUrl)) { + processedUrls.remove(decodedUrl) + Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") + return null + } + + Timber.tag("PhishingAndMalwareDetector").d("shouldIntercept $decodedUrl, referer ${request.requestHeaders["Referer"]}") + + if (request.isForMainFrame) { + if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { + return WebResourceResponse(null, null, null) + } + processedUrls.add(decodedUrl) + } else if ( + isForIframe(request) + ) { + if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { + withContext(dispatchers.main()) { + webView.stopLoading() + } + return WebResourceResponse(null, null, null) + } + processedUrls.add(decodedUrl) + } + return null + } + + override fun shouldOverrideUrlLoading( + url: Uri, + webView: WebView, + isForMainFrame: Boolean, + isRedirect: Boolean, + onSiteBlockedAsync: () -> Unit, + ): Boolean { + if (!isFeatureEnabled) { + return false + } + val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() + + if (processedUrls.contains(decodedUrl)) { + processedUrls.remove(decodedUrl) + Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") + return false + } + + Timber.tag("PhishingAndMalwareDetector").d("shouldOverrideUrlLoading $decodedUrl") + + if (isForMainFrame) { + if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { + return true + } + processedUrls.add(decodedUrl) + } + return false + } + + private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" || + request.url.path?.contains("/embed/") == true || + request.url.path?.contains("/iframe/") == true || + request.requestHeaders["Accept"]?.contains("text/html") == true + + override fun onPageLoadStarted() { + processedUrls.clear() + } } From ca13d27e60052d610a07763a87ed00e5a279f7f4 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 9 Dec 2024 11:11:59 +0100 Subject: [PATCH 02/12] Check if request belongs to current page --- .../app/browser/BrowserWebViewClient.kt | 2 +- .../app/browser/WebViewRequestInterceptor.kt | 3 ++- .../api/MaliciousSiteProtection.kt | 4 +--- .../impl/RealMaliciousSiteProtection.kt | 21 +++++-------------- 4 files changed, 9 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 23b7edb1b6db..cc4be064a52f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -165,7 +165,7 @@ class BrowserWebViewClient @Inject constructor( Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url") webViewClientListener?.onShouldOverride() - if (phishingAndMalwareDetector.shouldOverrideUrlLoading(url, webView, isForMainFrame, isRedirect, onSiteBlockedAsync)) { + if (phishingAndMalwareDetector.shouldOverrideUrlLoading(url, webView.url?.toUri(), isForMainFrame, isRedirect, onSiteBlockedAsync)) { // TODO (cbarreiro): Handle site blocked synchronously return true } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 2241c5f72003..ee7c2e360da6 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -102,7 +102,8 @@ class WebViewRequestInterceptor( val onSiteBlockedAsync: () -> Unit = { // TODO (cbarreiro): Handle site blocked asynchronously } - maliciousSiteProtection.shouldIntercept(request, webView, documentUri, onSiteBlockedAsync)?.let { + + maliciousSiteProtection.shouldIntercept(request, documentUri, onSiteBlockedAsync)?.let { // TODO (cbarreiro): Handle site blocked synchronously return it } diff --git a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt index 8d2f97cd67dc..46111f9a7d20 100644 --- a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt @@ -19,12 +19,10 @@ package com.duckduckgo.malicioussiteprotection.api import android.net.Uri import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse -import android.webkit.WebView interface MaliciousSiteProtection { suspend fun shouldIntercept( request: WebResourceRequest, - webView: WebView, documentUri: Uri?, onSiteBlockedAsync: () -> Unit, ): WebResourceResponse? @@ -33,7 +31,7 @@ interface MaliciousSiteProtection { fun shouldOverrideUrlLoading( url: Uri, - webView: WebView, + webViewUrl: Uri?, isForMainFrame: Boolean, isRedirect: Boolean, onSiteBlockedAsync: () -> Unit, diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index 1adfdbcd1093..cfac9d01e46e 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -19,7 +19,6 @@ package com.duckduckgo.malicioussiteprotection.impl import android.net.Uri import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse -import android.webkit.WebView import androidx.core.net.toUri import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.di.IsMainProcess @@ -33,7 +32,6 @@ import java.net.URLDecoder import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import org.json.JSONObject import timber.log.Timber @@ -75,13 +73,13 @@ class RealMaliciousSiteProtection @Inject constructor( private val processedUrls = mutableListOf() private fun shouldIntercept(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean { + Timber.tag("PhishingAndMalwareDetector").d("shouldIntercept $url") // TODO (cbarreiro): Implement the logic to check if the URL is malicious return false } override suspend fun shouldIntercept( request: WebResourceRequest, - webView: WebView, documentUri: Uri?, onSiteBlockedAsync: () -> Unit, ): WebResourceResponse? { @@ -104,20 +102,13 @@ class RealMaliciousSiteProtection @Inject constructor( return null } - Timber.tag("PhishingAndMalwareDetector").d("shouldIntercept $decodedUrl, referer ${request.requestHeaders["Referer"]}") - - if (request.isForMainFrame) { + if (request.isForMainFrame && decodedUrl.toUri() == documentUri) { if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { return WebResourceResponse(null, null, null) } processedUrls.add(decodedUrl) - } else if ( - isForIframe(request) - ) { + } else if (isForIframe(request) && documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host) { if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { - withContext(dispatchers.main()) { - webView.stopLoading() - } return WebResourceResponse(null, null, null) } processedUrls.add(decodedUrl) @@ -127,7 +118,7 @@ class RealMaliciousSiteProtection @Inject constructor( override fun shouldOverrideUrlLoading( url: Uri, - webView: WebView, + webViewUrl: Uri?, isForMainFrame: Boolean, isRedirect: Boolean, onSiteBlockedAsync: () -> Unit, @@ -143,9 +134,7 @@ class RealMaliciousSiteProtection @Inject constructor( return false } - Timber.tag("PhishingAndMalwareDetector").d("shouldOverrideUrlLoading $decodedUrl") - - if (isForMainFrame) { + if (isForMainFrame && decodedUrl.toUri() == webViewUrl) { if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { return true } From 8d1eaefcca1fc524531a06dea4b78c07cffb6da4 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 9 Dec 2024 11:34:38 +0100 Subject: [PATCH 03/12] Add tests --- .../build.gradle | 2 + .../impl/RealMaliciousSiteProtection.kt | 4 +- .../impl/RealMaliciousSiteProtectionTest.kt | 114 ++++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt diff --git a/malicious-site-protection/malicious-site-protection-impl/build.gradle b/malicious-site-protection/malicious-site-protection-impl/build.gradle index 8e5767047da1..c9454a21f7aa 100644 --- a/malicious-site-protection/malicious-site-protection-impl/build.gradle +++ b/malicious-site-protection/malicious-site-protection-impl/build.gradle @@ -42,11 +42,13 @@ dependencies { implementation Google.android.material + testImplementation AndroidX.test.ext.junit testImplementation Testing.junit4 testImplementation "org.mockito.kotlin:mockito-kotlin:_" testImplementation project(path: ':common-test') testImplementation CashApp.turbine testImplementation Testing.robolectric + testImplementation project(':feature-toggles-test') testImplementation(KotlinX.coroutines.test) { // https://github.com/Kotlin/kotlinx.coroutines/issues/2023 // conflicts with mockito due to direct inclusion of byte buddy diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index cfac9d01e46e..0e307d5ade25 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -19,6 +19,7 @@ package com.duckduckgo.malicioussiteprotection.impl import android.net.Uri import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse +import androidx.annotation.VisibleForTesting import androidx.core.net.toUri import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.di.IsMainProcess @@ -70,7 +71,8 @@ class RealMaliciousSiteProtection @Inject constructor( } } - private val processedUrls = mutableListOf() + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + val processedUrls = mutableListOf() private fun shouldIntercept(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean { Timber.tag("PhishingAndMalwareDetector").d("shouldIntercept $url") diff --git a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt new file mode 100644 index 000000000000..afc6a9a5d554 --- /dev/null +++ b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt @@ -0,0 +1,114 @@ +import android.net.Uri +import android.webkit.WebResourceRequest +import androidx.core.net.toUri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.duckduckgo.common.test.CoroutineTestRule +import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory +import com.duckduckgo.feature.toggles.api.Toggle +import com.duckduckgo.malicioussiteprotection.impl.MaliciousSiteProtectionFeature +import com.duckduckgo.malicioussiteprotection.impl.RealMaliciousSiteProtection +import junit.framework.TestCase.assertTrue +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.* +import org.mockito.kotlin.whenever + +@RunWith(AndroidJUnit4::class) +class RealMaliciousSiteProtectionTest { + + private val maliciousSiteProtectionFeature = FakeFeatureToggleFactory.create(MaliciousSiteProtectionFeature::class.java) + + private lateinit var testee: RealMaliciousSiteProtection + + @get:Rule + var coroutineTestRule = CoroutineTestRule() + + @Before + fun setup() { + testee = RealMaliciousSiteProtection( + coroutineTestRule.testDispatcherProvider, + maliciousSiteProtectionFeature, + true, + coroutineTestRule.testScope, + ) + } + + @Test + fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest { + val url = mock(Uri::class.java) + whenever(url.toString()).thenReturn("http://example.com") + maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(false)) + testee.onPrivacyConfigDownloaded() + + val result = testee.shouldOverrideUrlLoading(url, null, true, false) {} + assertFalse(result) + } + + @Test + fun `shouldOverrideUrlLoading returns false when for main frame`() = runTest { + val url = mock(Uri::class.java) + whenever(url.toString()).thenReturn("http://malicious.com") + maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) + testee.onPrivacyConfigDownloaded() + + val result = testee.shouldOverrideUrlLoading(url, "http://malicious.com".toUri(), true, false) {} + assertFalse(result) + } + + @Test + fun `shouldOverrideUrlLoading returns false when url is already processed`() = runTest { + val url = mock(Uri::class.java) + whenever(url.toString()).thenReturn("http://example.com") + maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) + testee.processedUrls.add("http://example.com") + testee.onPrivacyConfigDownloaded() + + val result = testee.shouldOverrideUrlLoading(url, "http://example.com".toUri(), true, false) {} + assertFalse(result) + } + + @Test + fun `shouldOverrideUrlLoading returns false when for iframe`() = runTest { + val url = mock(Uri::class.java) + whenever(url.toString()).thenReturn("http://malicious.com") + maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) + testee.onPrivacyConfigDownloaded() + + val result = testee.shouldOverrideUrlLoading(url, "http://example.com".toUri(), false, false) {} + assertFalse(result) + } + + @Test + fun `shouldInterceptRequest returns null when feature is disabled`() = runTest { + val request = mock(WebResourceRequest::class.java) + whenever(request.url).thenReturn("http://example.com".toUri()) + maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(false)) + testee.onPrivacyConfigDownloaded() + + val result = testee.shouldIntercept(request, null) {} + assertNull(result) + } + + @Test + fun `shouldInterceptRequest returns null when feature is enabled`() = runTest { + val request = mock(WebResourceRequest::class.java) + whenever(request.url).thenReturn("http://malicious.com".toUri()) + maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) + testee.onPrivacyConfigDownloaded() + + val result = testee.shouldIntercept(request, "http://malicious.com".toUri()) {} + assertNull(result) + } + + @Test + fun `onPageLoadStarted clears processedUrls`() = runTest { + testee.processedUrls.add("http://example.com") + testee.onPageLoadStarted() + assertTrue(testee.processedUrls.isEmpty()) + } +} From bb939651a899cf2ab1534debae012040a85fa8a9 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 9 Dec 2024 14:47:04 +0100 Subject: [PATCH 04/12] Suppress DenyListedApi --- .../impl/RealMaliciousSiteProtectionTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt index afc6a9a5d554..ff83e21b4c9d 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt @@ -1,3 +1,4 @@ +import android.annotation.SuppressLint import android.net.Uri import android.webkit.WebResourceRequest import androidx.core.net.toUri @@ -18,6 +19,7 @@ import org.junit.runner.RunWith import org.mockito.Mockito.* import org.mockito.kotlin.whenever +@SuppressLint("DenyListedApi") @RunWith(AndroidJUnit4::class) class RealMaliciousSiteProtectionTest { From 56c0b3e15af6615b6c2dbee8a0af4857c7655da9 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Tue, 10 Dec 2024 10:47:39 +0100 Subject: [PATCH 05/12] Make maliciousSiteProtection webView agnostic --- .../app/browser/BrowserWebViewClientTest.kt | 4 +- .../browser/WebViewRequestInterceptorTest.kt | 74 +++++----- .../UrlExtractingWebViewClientTest.kt | 4 +- .../referencetests/DomainsReferenceTest.kt | 6 +- .../referencetests/SurrogatesReferenceTest.kt | 6 +- .../app/browser/BrowserWebViewClient.kt | 24 +++- .../app/browser/WebViewRequestInterceptor.kt | 8 +- .../app/browser/di/BrowserModule.kt | 4 +- .../UrlExtractingWebViewClient.kt | 4 +- ...lMaliciousSiteBlockerWebViewIntegration.kt | 112 +++++++++++++++ ...iciousSiteBlockerWebViewIntegrationTest.kt | 131 ++++++++++++++++++ .../MaliciousSiteBlockerWebViewIntegration.kt | 39 ++++++ .../api/MaliciousSiteProtection.kt | 17 +-- .../impl/RealMaliciousSiteProtection.kt | 93 +------------ .../impl/RealMaliciousSiteProtectionTest.kt | 116 ---------------- 15 files changed, 365 insertions(+), 277 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt create mode 100644 app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt create mode 100644 browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt delete mode 100644 malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index a796fc873d99..ff942c065724 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -66,6 +66,7 @@ import com.duckduckgo.autoconsent.api.Autoconsent import com.duckduckgo.autofill.api.BrowserAutofill import com.duckduckgo.autofill.api.InternalTestUserChecker import com.duckduckgo.browser.api.JsInjectorPlugin +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.browser.api.WebViewVersionProvider import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.utils.CurrentTimeProvider @@ -79,7 +80,6 @@ import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Off import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Unavailable import com.duckduckgo.history.api.NavigationHistory -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.AmpLinks import com.duckduckgo.subscriptions.api.Subscriptions import com.duckduckgo.user.agent.api.ClientBrandHintProvider @@ -153,7 +153,7 @@ class BrowserWebViewClientTest { private val mockUriLoadedManager: UriLoadedManager = mock() private val mockAndroidBrowserConfigFeature: AndroidBrowserConfigFeature = mock() private val mockAndroidFeaturesHeaderPlugin = AndroidFeaturesHeaderPlugin(mockDuckDuckGoUrlDetector, mockAndroidBrowserConfigFeature, mock()) - private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() + private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock() @UiThreadTest @Before diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt index 8bccdddac934..ad115fcf07c6 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt @@ -42,11 +42,11 @@ import com.duckduckgo.app.trackerdetection.TrackerDetector import com.duckduckgo.app.trackerdetection.model.TrackerStatus import com.duckduckgo.app.trackerdetection.model.TrackerType import com.duckduckgo.app.trackerdetection.model.TrackingEvent +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle import com.duckduckgo.httpsupgrade.api.HttpsUpgrader -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.impl.features.gpc.RealGpc.Companion.GPC_HEADER import com.duckduckgo.request.filterer.api.RequestFilterer @@ -98,7 +98,7 @@ class WebViewRequestInterceptorTest { fakeToggle, fakeUserAllowListRepository, ) - private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() + private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock() private var webView: WebView = mock() @@ -130,7 +130,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) assertCancelledResponse(response) @@ -143,7 +143,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -157,7 +157,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -172,7 +172,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -187,7 +187,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) verify(mockHttpsUpgrader).upgrade(any()) @@ -203,7 +203,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -217,7 +217,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) verify(mockDuckPlayer).intercept(any(), any(), any()) @@ -231,7 +231,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -245,7 +245,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -259,7 +259,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) assertRequestCanContinueToLoad(response) @@ -272,7 +272,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duckduckgo.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -286,7 +286,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "donttrack.us/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -300,7 +300,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "spreadprivacy.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -314,7 +314,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duckduckhack.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -328,7 +328,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "privatebrowsingmyths.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -342,7 +342,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duck.co/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -359,7 +359,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockListener, ) @@ -377,7 +377,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockListener, ) @@ -395,7 +395,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -418,7 +418,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -442,7 +442,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -457,7 +457,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -474,7 +474,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -492,7 +492,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -510,7 +510,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -528,7 +528,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -546,7 +546,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -563,7 +563,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -580,7 +580,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -597,7 +597,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -614,7 +614,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -674,7 +674,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -713,7 +713,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -735,7 +735,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -755,7 +755,7 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt index b97b21da6bde..fd521dfdf4af 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt @@ -24,9 +24,9 @@ import com.duckduckgo.app.browser.* import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.cookies.api.CookieManagerProvider -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Before @@ -51,7 +51,7 @@ class UrlExtractingWebViewClientTest { private val thirdPartyCookieManager: ThirdPartyCookieManager = mock() private val urlExtractor: DOMUrlExtractor = mock() private val mockWebView: WebView = mock() - private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() + private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock() @UiThreadTest @Before diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt index e9efcba2c2b3..50f945aa1b23 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt @@ -52,12 +52,12 @@ import com.duckduckgo.app.trackerdetection.db.TdsCnameEntityDao import com.duckduckgo.app.trackerdetection.db.TdsDomainEntityDao import com.duckduckgo.app.trackerdetection.db.TdsEntityDao import com.duckduckgo.app.trackerdetection.db.WebTrackersBlockedDao +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.test.FileUtilities import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle import com.duckduckgo.httpsupgrade.api.HttpsUpgrader -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.ContentBlocking import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.api.TrackerAllowlist @@ -120,7 +120,7 @@ class DomainsReferenceTest(private val testCase: TestCase) { ) private val mockGpc: Gpc = mock() private val mockAdClickManager: AdClickManager = mock() - private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() + private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock() companion object { private val moshi = Moshi.Builder().add(ActionJsonAdapter()).build() @@ -200,7 +200,7 @@ class DomainsReferenceTest(private val testCase: TestCase) { request = mockRequest, documentUri = testCase.siteURL.toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt index 1615945a7efa..8d6e3015751f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt @@ -50,12 +50,12 @@ import com.duckduckgo.app.trackerdetection.db.TdsCnameEntityDao import com.duckduckgo.app.trackerdetection.db.TdsDomainEntityDao import com.duckduckgo.app.trackerdetection.db.TdsEntityDao import com.duckduckgo.app.trackerdetection.db.WebTrackersBlockedDao +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.test.FileUtilities import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle import com.duckduckgo.httpsupgrade.api.HttpsUpgrader -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.ContentBlocking import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.api.TrackerAllowlist @@ -116,7 +116,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) { private val mockGpc: Gpc = mock() private val mockAdClickManager: AdClickManager = mock() private val mockCloakedCnameDetector: CloakedCnameDetector = mock() - private val mockMaliciousSiteProtection: MaliciousSiteProtection = mock() + private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock() companion object { private val moshi = Moshi.Builder().add(ActionJsonAdapter()).build() @@ -189,7 +189,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) { request = mockRequest, documentUri = testCase.siteURL.toUri(), webView = webView, - maliciousSiteProtection = mockMaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index cc4be064a52f..e97175e533b1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -66,6 +66,7 @@ import com.duckduckgo.autoconsent.api.Autoconsent import com.duckduckgo.autofill.api.BrowserAutofill import com.duckduckgo.autofill.api.InternalTestUserChecker import com.duckduckgo.browser.api.JsInjectorPlugin +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.CurrentTimeProvider import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.plugins.PluginPoint @@ -76,7 +77,6 @@ import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.ENABLED import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On import com.duckduckgo.duckplayer.impl.DUCK_PLAYER_OPEN_IN_YOUTUBE_PATH import com.duckduckgo.history.api.NavigationHistory -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.AmpLinks import com.duckduckgo.subscriptions.api.Subscriptions import com.duckduckgo.user.agent.api.ClientBrandHintProvider @@ -118,7 +118,7 @@ class BrowserWebViewClient @Inject constructor( private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector, private val uriLoadedManager: UriLoadedManager, private val androidFeaturesHeaderPlugin: AndroidFeaturesHeaderPlugin, - private val phishingAndMalwareDetector: MaliciousSiteProtection, + private val maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, ) : WebViewClient() { var webViewClientListener: WebViewClientListener? = null @@ -165,7 +165,15 @@ class BrowserWebViewClient @Inject constructor( Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url") webViewClientListener?.onShouldOverride() - if (phishingAndMalwareDetector.shouldOverrideUrlLoading(url, webView.url?.toUri(), isForMainFrame, isRedirect, onSiteBlockedAsync)) { + if (runBlocking { + maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading( + url, + webView.url?.toUri(), + isForMainFrame, + onSiteBlockedAsync, + ) + } + ) { // TODO (cbarreiro): Handle site blocked synchronously return true } @@ -419,7 +427,7 @@ class BrowserWebViewClient @Inject constructor( // See https://app.asana.com/0/0/1206159443951489/f (WebView limitations) if (it != ABOUT_BLANK && start == null) { start = currentTimeProvider.elapsedRealtime() - phishingAndMalwareDetector.onPageLoadStarted() + maliciousSiteProtectionWebViewIntegration.onPageLoadStarted() } handleMediaPlayback(webView, it) autoconsent.injectAutoconsent(webView, url) @@ -522,7 +530,13 @@ class BrowserWebViewClient @Inject constructor( loginDetector.onEvent(WebNavigationEvent.ShouldInterceptRequest(webView, request)) } Timber.v("Intercepting resource ${request.url} type:${request.method} on page $documentUrl") - requestInterceptor.shouldIntercept(request, webView, documentUrl?.toUri(), phishingAndMalwareDetector, webViewClientListener) + requestInterceptor.shouldIntercept( + request, + webView, + documentUrl?.toUri(), + maliciousSiteProtectionWebViewIntegration, + webViewClientListener, + ) } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index ee7c2e360da6..0adaad641815 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -29,13 +29,13 @@ import com.duckduckgo.app.trackerdetection.CloakedCnameDetector import com.duckduckgo.app.trackerdetection.TrackerDetector import com.duckduckgo.app.trackerdetection.model.TrackerStatus import com.duckduckgo.app.trackerdetection.model.TrackingEvent +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.AppUrl import com.duckduckgo.common.utils.DefaultDispatcherProvider import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.isHttp import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.httpsupgrade.api.HttpsUpgrader -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.request.filterer.api.RequestFilterer import com.duckduckgo.user.agent.api.UserAgentProvider @@ -49,7 +49,7 @@ interface RequestInterceptor { request: WebResourceRequest, webView: WebView, documentUri: Uri?, - maliciousSiteProtection: MaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, webViewClientListener: WebViewClientListener?, ): WebResourceResponse? @@ -94,7 +94,7 @@ class WebViewRequestInterceptor( request: WebResourceRequest, webView: WebView, documentUri: Uri?, - maliciousSiteProtection: MaliciousSiteProtection, + maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, webViewClientListener: WebViewClientListener?, ): WebResourceResponse? { val url: Uri? = request.url @@ -103,7 +103,7 @@ class WebViewRequestInterceptor( // TODO (cbarreiro): Handle site blocked asynchronously } - maliciousSiteProtection.shouldIntercept(request, documentUri, onSiteBlockedAsync)?.let { + maliciousSiteProtectionWebViewIntegration.shouldIntercept(request, documentUri, onSiteBlockedAsync)?.let { // TODO (cbarreiro): Handle site blocked synchronously return it } diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index 39b6e7004be7..b0a80118f83f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -72,6 +72,7 @@ import com.duckduckgo.app.surrogates.ResourceSurrogates import com.duckduckgo.app.tabs.ui.GridViewColumnCalculator import com.duckduckgo.app.trackerdetection.CloakedCnameDetector import com.duckduckgo.app.trackerdetection.TrackerDetector +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.cookies.api.CookieManagerProvider import com.duckduckgo.cookies.api.ThirdPartyCookieNames @@ -84,7 +85,6 @@ import com.duckduckgo.downloads.impl.FileDownloadCallback import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.experiments.api.VariantManager import com.duckduckgo.httpsupgrade.api.HttpsUpgrader -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.AmpLinks import com.duckduckgo.privacy.config.api.Gpc import com.duckduckgo.privacy.config.api.TrackingParameters @@ -122,7 +122,7 @@ class BrowserModule { @AppCoroutineScope appCoroutineScope: CoroutineScope, dispatcherProvider: DispatcherProvider, urlExtractor: DOMUrlExtractor, - maliciousSiteProtection: MaliciousSiteProtection, + maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration, ): UrlExtractingWebViewClient { return UrlExtractingWebViewClient( webViewHttpAuthStore, diff --git a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt index dd41f50332ce..63516ae4e464 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt @@ -28,9 +28,9 @@ import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationSt import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.cookies.api.CookieManagerProvider -import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import kotlinx.coroutines.* import timber.log.Timber @@ -43,7 +43,7 @@ class UrlExtractingWebViewClient( private val appCoroutineScope: CoroutineScope, private val dispatcherProvider: DispatcherProvider, private val urlExtractor: DOMUrlExtractor, - private val maliciousSiteProtection: MaliciousSiteProtection, + private val maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration, ) : WebViewClient() { var urlExtractionListener: UrlExtractionListener? = null diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt new file mode 100644 index 000000000000..6bd1ed7183f3 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * 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. + */ + +package com.duckduckgo.app.browser.webview + +import android.net.Uri +import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse +import androidx.annotation.VisibleForTesting +import androidx.core.net.toUri +import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration +import com.duckduckgo.di.scopes.AppScope +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection +import com.squareup.anvil.annotations.ContributesBinding +import java.net.URLDecoder +import javax.inject.Inject +import timber.log.Timber + +@ContributesBinding(AppScope::class) +class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( + private val maliciousSiteProtection: MaliciousSiteProtection, +) : MaliciousSiteBlockerWebViewIntegration { + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + val processedUrls = mutableListOf() + + override suspend fun shouldIntercept( + request: WebResourceRequest, + documentUri: Uri?, + onSiteBlockedAsync: () -> Unit, + ): WebResourceResponse? { + if (!maliciousSiteProtection.isFeatureEnabled) { + return null + } + val url = request.url.let { + if (it.fragment != null) { + it.buildUpon().fragment(null).build() + } else { + it + } + } + + val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() + + if (processedUrls.contains(decodedUrl)) { + processedUrls.remove(decodedUrl) + Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") + return null + } + + if (request.isForMainFrame && decodedUrl.toUri() == documentUri) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) { + return WebResourceResponse(null, null, null) + } + processedUrls.add(decodedUrl) + } else if (isForIframe(request) && documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) { + return WebResourceResponse(null, null, null) + } + processedUrls.add(decodedUrl) + } + return null + } + + override suspend fun shouldOverrideUrlLoading( + url: Uri, + webViewUrl: Uri?, + isForMainFrame: Boolean, + onSiteBlockedAsync: () -> Unit, + ): Boolean { + if (!maliciousSiteProtection.isFeatureEnabled) { + return false + } + val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() + + if (processedUrls.contains(decodedUrl)) { + processedUrls.remove(decodedUrl) + Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") + return false + } + + if (isForMainFrame && decodedUrl.toUri() == webViewUrl) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) { + return true + } + processedUrls.add(decodedUrl) + } + return false + } + + private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" || + request.url.path?.contains("/embed/") == true || + request.url.path?.contains("/iframe/") == true || + request.requestHeaders["Accept"]?.contains("text/html") == true + + override fun onPageLoadStarted() { + processedUrls.clear() + } +} diff --git a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt new file mode 100644 index 000000000000..e1ee15280d88 --- /dev/null +++ b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt @@ -0,0 +1,131 @@ +package com.duckduckgo.app.browser.webview + +import android.webkit.WebResourceRequest +import androidx.core.net.toUri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection +import junit.framework.TestCase.assertTrue +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.mock +import org.mockito.kotlin.any +import org.mockito.kotlin.whenever + +@RunWith(AndroidJUnit4::class) +class RealMaliciousSiteBlockerWebViewIntegrationTest { + + private lateinit var testee: RealMaliciousSiteBlockerWebViewIntegration + private val maliciousSiteProtection: MaliciousSiteProtection = mock(MaliciousSiteProtection::class.java) + private val maliciousUri = "http://malicious.com".toUri() + private val exampleUri = "http://example.com".toUri() + + @Before + fun setup() { + testee = RealMaliciousSiteBlockerWebViewIntegration(maliciousSiteProtection) + whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(true) + } + + @Test + fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest { + whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(false) + + val result = testee.shouldOverrideUrlLoading(exampleUri, null, true) {} + assertFalse(result) + } + + @Test + fun `shouldInterceptRequest returns null when feature is disabled`() = runTest { + val request = mock(WebResourceRequest::class.java) + whenever(request.url).thenReturn(exampleUri) + whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(false) + + val result = testee.shouldIntercept(request, null) {} + assertNull(result) + } + + @Test + fun `shouldOverrideUrlLoading returns false when url is already processed`() = runTest { + testee.processedUrls.add(exampleUri.toString()) + + val result = testee.shouldOverrideUrlLoading(exampleUri, exampleUri, true) {} + assertFalse(result) + } + + @Test + fun `shouldInterceptRequest returns result when feature is enabled, is malicious, and is mainframe`() = runTest { + val request = mock(WebResourceRequest::class.java) + whenever(request.url).thenReturn(maliciousUri) + whenever(request.isForMainFrame).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldIntercept(request, maliciousUri) {} + assertNotNull(result) + } + + @Test + fun `shouldInterceptRequest returns result when feature is enabled, is malicious, and is iframe`() = runTest { + val request = mock(WebResourceRequest::class.java) + whenever(request.url).thenReturn(maliciousUri) + whenever(request.isForMainFrame).thenReturn(true) + whenever(request.requestHeaders).thenReturn(mapOf("Sec-Fetch-Dest" to "iframe")) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldIntercept(request, maliciousUri) {} + assertNotNull(result) + } + + @Test + fun `shouldInterceptRequest returns null when feature is enabled, is malicious, and is not mainframe nor iframe`() = runTest { + val request = mock(WebResourceRequest::class.java) + whenever(request.url).thenReturn(maliciousUri) + whenever(request.isForMainFrame).thenReturn(false) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldIntercept(request, maliciousUri) {} + assertNull(result) + } + + @Test + fun `shouldOverride returns false when feature is enabled, is malicious, and is not mainframe`() = runTest { + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} + assertFalse(result) + } + + @Test + fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe`() = runTest { + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, true) {} + assertTrue(result) + } + + @Test + fun `shouldOverride returns false when feature is enabled, is malicious, and not mainframe nor iframe`() = runTest { + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} + assertFalse(result) + } + + @Test + fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe but webView has different host`() = runTest { + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + + val result = testee.shouldOverrideUrlLoading(maliciousUri, exampleUri, true) {} + assertFalse(result) + } + + @Test + fun `onPageLoadStarted clears processedUrls`() = runTest { + testee.processedUrls.add(exampleUri.toString()) + testee.onPageLoadStarted() + assertTrue(testee.processedUrls.isEmpty()) + } +} diff --git a/browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt b/browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt new file mode 100644 index 000000000000..4b9a2e8c0b75 --- /dev/null +++ b/browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * 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. + */ + +package com.duckduckgo.browser.api + +import android.net.Uri +import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse + +interface MaliciousSiteBlockerWebViewIntegration { + + suspend fun shouldIntercept( + request: WebResourceRequest, + documentUri: Uri?, + onSiteBlockedAsync: () -> Unit, + ): WebResourceResponse? + + suspend fun shouldOverrideUrlLoading( + url: Uri, + webViewUrl: Uri?, + isForMainFrame: Boolean, + onSiteBlockedAsync: () -> Unit, + ): Boolean + + fun onPageLoadStarted() +} diff --git a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt index 46111f9a7d20..0ebd2e4e6b26 100644 --- a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt @@ -17,23 +17,10 @@ package com.duckduckgo.malicioussiteprotection.api import android.net.Uri -import android.webkit.WebResourceRequest -import android.webkit.WebResourceResponse interface MaliciousSiteProtection { - suspend fun shouldIntercept( - request: WebResourceRequest, - documentUri: Uri?, - onSiteBlockedAsync: () -> Unit, - ): WebResourceResponse? - fun onPageLoadStarted() + suspend fun isMalicious(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean - fun shouldOverrideUrlLoading( - url: Uri, - webViewUrl: Uri?, - isForMainFrame: Boolean, - isRedirect: Boolean, - onSiteBlockedAsync: () -> Unit, - ): Boolean + val isFeatureEnabled: Boolean } diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index 0e307d5ade25..7e6a43b8fa56 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -17,10 +17,6 @@ package com.duckduckgo.malicioussiteprotection.impl import android.net.Uri -import android.webkit.WebResourceRequest -import android.webkit.WebResourceResponse -import androidx.annotation.VisibleForTesting -import androidx.core.net.toUri import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.di.IsMainProcess import com.duckduckgo.common.utils.DispatcherProvider @@ -29,7 +25,6 @@ import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding -import java.net.URLDecoder import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -45,7 +40,10 @@ class RealMaliciousSiteProtection @Inject constructor( @AppCoroutineScope private val appCoroutineScope: CoroutineScope, ) : MaliciousSiteProtection, PrivacyConfigCallbackPlugin { - private var isFeatureEnabled = false + private var _isFeatureEnabled = false + override val isFeatureEnabled: Boolean + get() = _isFeatureEnabled + private var hashPrefixUpdateFrequency = 20L private var filterSetUpdateFrequency = 720L @@ -61,7 +59,7 @@ class RealMaliciousSiteProtection @Inject constructor( private fun loadToMemory() { appCoroutineScope.launch(dispatchers.io()) { - isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() + _isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() maliciousSiteProtectionFeature.self().getSettings()?.let { JSONObject(it).let { settings -> hashPrefixUpdateFrequency = settings.getLong("hashPrefixUpdateFrequency") @@ -71,86 +69,9 @@ class RealMaliciousSiteProtection @Inject constructor( } } - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - val processedUrls = mutableListOf() - - private fun shouldIntercept(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean { - Timber.tag("PhishingAndMalwareDetector").d("shouldIntercept $url") + override suspend fun isMalicious(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean { + Timber.tag("MaliciousSiteProtection").d("isMalicious $url") // TODO (cbarreiro): Implement the logic to check if the URL is malicious return false } - - override suspend fun shouldIntercept( - request: WebResourceRequest, - documentUri: Uri?, - onSiteBlockedAsync: () -> Unit, - ): WebResourceResponse? { - if (!isFeatureEnabled) { - return null - } - val url = request.url.let { - if (it.fragment != null) { - it.buildUpon().fragment(null).build() - } else { - it - } - } - - val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() - - if (processedUrls.contains(decodedUrl)) { - processedUrls.remove(decodedUrl) - Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") - return null - } - - if (request.isForMainFrame && decodedUrl.toUri() == documentUri) { - if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { - return WebResourceResponse(null, null, null) - } - processedUrls.add(decodedUrl) - } else if (isForIframe(request) && documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host) { - if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { - return WebResourceResponse(null, null, null) - } - processedUrls.add(decodedUrl) - } - return null - } - - override fun shouldOverrideUrlLoading( - url: Uri, - webViewUrl: Uri?, - isForMainFrame: Boolean, - isRedirect: Boolean, - onSiteBlockedAsync: () -> Unit, - ): Boolean { - if (!isFeatureEnabled) { - return false - } - val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() - - if (processedUrls.contains(decodedUrl)) { - processedUrls.remove(decodedUrl) - Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") - return false - } - - if (isForMainFrame && decodedUrl.toUri() == webViewUrl) { - if (shouldIntercept(decodedUrl.toUri(), onSiteBlockedAsync)) { - return true - } - processedUrls.add(decodedUrl) - } - return false - } - - private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" || - request.url.path?.contains("/embed/") == true || - request.url.path?.contains("/iframe/") == true || - request.requestHeaders["Accept"]?.contains("text/html") == true - - override fun onPageLoadStarted() { - processedUrls.clear() - } } diff --git a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt deleted file mode 100644 index ff83e21b4c9d..000000000000 --- a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtectionTest.kt +++ /dev/null @@ -1,116 +0,0 @@ -import android.annotation.SuppressLint -import android.net.Uri -import android.webkit.WebResourceRequest -import androidx.core.net.toUri -import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.duckduckgo.common.test.CoroutineTestRule -import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory -import com.duckduckgo.feature.toggles.api.Toggle -import com.duckduckgo.malicioussiteprotection.impl.MaliciousSiteProtectionFeature -import com.duckduckgo.malicioussiteprotection.impl.RealMaliciousSiteProtection -import junit.framework.TestCase.assertTrue -import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertFalse -import org.junit.Assert.assertNull -import org.junit.Before -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.mockito.Mockito.* -import org.mockito.kotlin.whenever - -@SuppressLint("DenyListedApi") -@RunWith(AndroidJUnit4::class) -class RealMaliciousSiteProtectionTest { - - private val maliciousSiteProtectionFeature = FakeFeatureToggleFactory.create(MaliciousSiteProtectionFeature::class.java) - - private lateinit var testee: RealMaliciousSiteProtection - - @get:Rule - var coroutineTestRule = CoroutineTestRule() - - @Before - fun setup() { - testee = RealMaliciousSiteProtection( - coroutineTestRule.testDispatcherProvider, - maliciousSiteProtectionFeature, - true, - coroutineTestRule.testScope, - ) - } - - @Test - fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest { - val url = mock(Uri::class.java) - whenever(url.toString()).thenReturn("http://example.com") - maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(false)) - testee.onPrivacyConfigDownloaded() - - val result = testee.shouldOverrideUrlLoading(url, null, true, false) {} - assertFalse(result) - } - - @Test - fun `shouldOverrideUrlLoading returns false when for main frame`() = runTest { - val url = mock(Uri::class.java) - whenever(url.toString()).thenReturn("http://malicious.com") - maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) - testee.onPrivacyConfigDownloaded() - - val result = testee.shouldOverrideUrlLoading(url, "http://malicious.com".toUri(), true, false) {} - assertFalse(result) - } - - @Test - fun `shouldOverrideUrlLoading returns false when url is already processed`() = runTest { - val url = mock(Uri::class.java) - whenever(url.toString()).thenReturn("http://example.com") - maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) - testee.processedUrls.add("http://example.com") - testee.onPrivacyConfigDownloaded() - - val result = testee.shouldOverrideUrlLoading(url, "http://example.com".toUri(), true, false) {} - assertFalse(result) - } - - @Test - fun `shouldOverrideUrlLoading returns false when for iframe`() = runTest { - val url = mock(Uri::class.java) - whenever(url.toString()).thenReturn("http://malicious.com") - maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) - testee.onPrivacyConfigDownloaded() - - val result = testee.shouldOverrideUrlLoading(url, "http://example.com".toUri(), false, false) {} - assertFalse(result) - } - - @Test - fun `shouldInterceptRequest returns null when feature is disabled`() = runTest { - val request = mock(WebResourceRequest::class.java) - whenever(request.url).thenReturn("http://example.com".toUri()) - maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(false)) - testee.onPrivacyConfigDownloaded() - - val result = testee.shouldIntercept(request, null) {} - assertNull(result) - } - - @Test - fun `shouldInterceptRequest returns null when feature is enabled`() = runTest { - val request = mock(WebResourceRequest::class.java) - whenever(request.url).thenReturn("http://malicious.com".toUri()) - maliciousSiteProtectionFeature.self().setRawStoredState(Toggle.State(true)) - testee.onPrivacyConfigDownloaded() - - val result = testee.shouldIntercept(request, "http://malicious.com".toUri()) {} - assertNull(result) - } - - @Test - fun `onPageLoadStarted clears processedUrls`() = runTest { - testee.processedUrls.add("http://example.com") - testee.onPageLoadStarted() - assertTrue(testee.processedUrls.isEmpty()) - } -} From 762e22bd7404e84cc121c4bc7e81117c2d18c4ee Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Tue, 10 Dec 2024 16:06:46 +0100 Subject: [PATCH 06/12] Don't expose RC flag in MaliciousSiteProtection --- .../app/browser/BrowserWebViewClientTest.kt | 14 +++++-- ...lMaliciousSiteBlockerWebViewIntegration.kt | 38 +++++++++++++++++-- .../AndroidBrowserConfigFeature.kt | 10 +++++ ...iciousSiteBlockerWebViewIntegrationTest.kt | 29 +++++++++++--- .../api/MaliciousSiteProtection.kt | 2 - .../impl/RealMaliciousSiteProtection.kt | 6 +-- 6 files changed, 80 insertions(+), 19 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index ff942c065724..ac56d97e6e1a 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -157,7 +157,7 @@ class BrowserWebViewClientTest { @UiThreadTest @Before - fun setup() { + fun setup() = runTest { webView = TestWebView(context) whenever(mockDuckPlayer.observeShouldOpenInNewTab()).thenReturn(openInNewTabFlow) testee = BrowserWebViewClient( @@ -199,6 +199,7 @@ class BrowserWebViewClientTest { whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0) whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1") whenever(deviceInfo.appVersion).thenReturn("1") + whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any(), any())).thenReturn(false) } @UiThreadTest @@ -318,8 +319,8 @@ class BrowserWebViewClientTest { @Test fun whenOnReceivedHttpAuthRequestThenListenerNotified() { val mockHandler = mock() - val authenticationRequest = BasicAuthenticationRequest(mockHandler, EXAMPLE_URL, EXAMPLE_URL, EXAMPLE_URL) - testee.onReceivedHttpAuthRequest(webView, mockHandler, EXAMPLE_URL, EXAMPLE_URL) + val authenticationRequest = BasicAuthenticationRequest(mockHandler, "example.com", EXAMPLE_URL, EXAMPLE_URL) + testee.onReceivedHttpAuthRequest(webView, mockHandler, "example.com", EXAMPLE_URL) verify(listener).requiresAuthentication(authenticationRequest) } @@ -768,6 +769,7 @@ class BrowserWebViewClientTest { private fun getImmediatelyInvokedMockWebView(): WebView { val mockWebView = mock() whenever(mockWebView.originalUrl).thenReturn(EXAMPLE_URL) + whenever(mockWebView.url).thenReturn(EXAMPLE_URL) whenever(mockWebView.post(any())).thenAnswer { invocation -> invocation.getArgument(0, Runnable::class.java).run() null @@ -1074,6 +1076,10 @@ class BrowserWebViewClientTest { override fun getOriginalUrl(): String { return EXAMPLE_URL } + + override fun getUrl(): String { + return EXAMPLE_URL + } } private class FakePluginPoint : PluginPoint { @@ -1152,6 +1158,6 @@ class BrowserWebViewClientTest { } companion object { - const val EXAMPLE_URL = "example.com" + const val EXAMPLE_URL = "https://example.com" } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt index 6bd1ed7183f3..b621492409ab 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt @@ -21,28 +21,58 @@ import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse import androidx.annotation.VisibleForTesting import androidx.core.net.toUri +import com.duckduckgo.app.di.AppCoroutineScope +import com.duckduckgo.app.di.IsMainProcess +import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration +import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection +import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding +import com.squareup.anvil.annotations.ContributesMultibinding import java.net.URLDecoder import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch import timber.log.Timber -@ContributesBinding(AppScope::class) +@ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) +@ContributesBinding(AppScope::class, MaliciousSiteBlockerWebViewIntegration::class) class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( private val maliciousSiteProtection: MaliciousSiteProtection, -) : MaliciousSiteBlockerWebViewIntegration { + private val androidBrowserConfigFeature: AndroidBrowserConfigFeature, + private val dispatchers: DispatcherProvider, + @AppCoroutineScope private val appCoroutineScope: CoroutineScope, + @IsMainProcess private val isMainProcess: Boolean, +) : MaliciousSiteBlockerWebViewIntegration, PrivacyConfigCallbackPlugin { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) val processedUrls = mutableListOf() + private var isFeatureEnabled = false + + init { + if (isMainProcess) { + loadToMemory() + } + } + + private fun loadToMemory() { + appCoroutineScope.launch(dispatchers.io()) { + isFeatureEnabled = androidBrowserConfigFeature.enableMaliciousSiteProtection().isEnabled() + } + } + + override fun onPrivacyConfigDownloaded() { + loadToMemory() + } override suspend fun shouldIntercept( request: WebResourceRequest, documentUri: Uri?, onSiteBlockedAsync: () -> Unit, ): WebResourceResponse? { - if (!maliciousSiteProtection.isFeatureEnabled) { + if (!isFeatureEnabled) { return null } val url = request.url.let { @@ -81,7 +111,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( isForMainFrame: Boolean, onSiteBlockedAsync: () -> Unit, ): Boolean { - if (!maliciousSiteProtection.isFeatureEnabled) { + if (!isFeatureEnabled) { return false } val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() diff --git a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt index f555e8ccb068..593d6cc2b9d0 100644 --- a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt +++ b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt @@ -19,6 +19,7 @@ package com.duckduckgo.app.pixels.remoteconfig import com.duckduckgo.anvil.annotations.ContributesRemoteFeature import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.feature.toggles.api.Toggle +import com.duckduckgo.feature.toggles.api.Toggle.InternalAlwaysEnabled /** * This is the class that represents the browser feature flags @@ -91,4 +92,13 @@ interface AndroidBrowserConfigFeature { */ @Toggle.DefaultValue(false) fun webLocalStorage(): Toggle + + /** + * @return `true` when the remote config has the global "enableMaliciousSiteProtection" androidBrowserConfig + * sub-feature flag enabled + * If the remote feature is not present defaults to `false` + */ + @InternalAlwaysEnabled + @Toggle.DefaultValue(false) + fun enableMaliciousSiteProtection(): Toggle } diff --git a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt index e1ee15280d88..3ac09713ff25 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt @@ -3,6 +3,10 @@ package com.duckduckgo.app.browser.webview import android.webkit.WebResourceRequest import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature +import com.duckduckgo.common.test.CoroutineTestRule +import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory +import com.duckduckgo.feature.toggles.api.Toggle.State import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import junit.framework.TestCase.assertTrue import kotlinx.coroutines.test.runTest @@ -10,6 +14,7 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.mock @@ -19,20 +24,29 @@ import org.mockito.kotlin.whenever @RunWith(AndroidJUnit4::class) class RealMaliciousSiteBlockerWebViewIntegrationTest { - private lateinit var testee: RealMaliciousSiteBlockerWebViewIntegration + @get:Rule + var coroutineRule = CoroutineTestRule() + private val maliciousSiteProtection: MaliciousSiteProtection = mock(MaliciousSiteProtection::class.java) + private val fakeAndroidBrowserConfigFeature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java) private val maliciousUri = "http://malicious.com".toUri() private val exampleUri = "http://example.com".toUri() + private val testee = RealMaliciousSiteBlockerWebViewIntegration( + maliciousSiteProtection, + androidBrowserConfigFeature = fakeAndroidBrowserConfigFeature, + dispatchers = coroutineRule.testDispatcherProvider, + appCoroutineScope = coroutineRule.testScope, + isMainProcess = true, + ) @Before fun setup() { - testee = RealMaliciousSiteBlockerWebViewIntegration(maliciousSiteProtection) - whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(true) + updateFeatureEnabled(true) } @Test fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest { - whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(false) + updateFeatureEnabled(false) val result = testee.shouldOverrideUrlLoading(exampleUri, null, true) {} assertFalse(result) @@ -42,7 +56,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldInterceptRequest returns null when feature is disabled`() = runTest { val request = mock(WebResourceRequest::class.java) whenever(request.url).thenReturn(exampleUri) - whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(false) + updateFeatureEnabled(false) val result = testee.shouldIntercept(request, null) {} assertNull(result) @@ -128,4 +142,9 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { testee.onPageLoadStarted() assertTrue(testee.processedUrls.isEmpty()) } + + private fun updateFeatureEnabled(enabled: Boolean) { + fakeAndroidBrowserConfigFeature.enableMaliciousSiteProtection().setRawStoredState(State(enabled)) + testee.onPrivacyConfigDownloaded() + } } diff --git a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt index 0ebd2e4e6b26..0e4eb8d56ddc 100644 --- a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt @@ -21,6 +21,4 @@ import android.net.Uri interface MaliciousSiteProtection { suspend fun isMalicious(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean - - val isFeatureEnabled: Boolean } diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index 7e6a43b8fa56..c703412445d0 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -40,9 +40,7 @@ class RealMaliciousSiteProtection @Inject constructor( @AppCoroutineScope private val appCoroutineScope: CoroutineScope, ) : MaliciousSiteProtection, PrivacyConfigCallbackPlugin { - private var _isFeatureEnabled = false - override val isFeatureEnabled: Boolean - get() = _isFeatureEnabled + private var isFeatureEnabled = false private var hashPrefixUpdateFrequency = 20L private var filterSetUpdateFrequency = 720L @@ -59,7 +57,7 @@ class RealMaliciousSiteProtection @Inject constructor( private fun loadToMemory() { appCoroutineScope.launch(dispatchers.io()) { - _isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() + isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() maliciousSiteProtectionFeature.self().getSettings()?.let { JSONObject(it).let { settings -> hashPrefixUpdateFrequency = settings.getLong("hashPrefixUpdateFrequency") From 050b06f80275c513f036e944142453c820076790 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Tue, 10 Dec 2024 18:23:39 +0100 Subject: [PATCH 07/12] Do not make MaliciousSiteBlockerWebViewIntegration public --- .../app/browser/BrowserWebViewClientTest.kt | 2 +- .../browser/WebViewRequestInterceptorTest.kt | 1 - .../UrlExtractingWebViewClientTest.kt | 2 +- .../referencetests/DomainsReferenceTest.kt | 2 +- .../referencetests/SurrogatesReferenceTest.kt | 2 +- .../app/browser/BrowserWebViewClient.kt | 2 +- .../app/browser/WebViewRequestInterceptor.kt | 2 +- .../app/browser/di/BrowserModule.kt | 2 +- .../UrlExtractingWebViewClient.kt | 2 +- ...MaliciousSiteBlockerWebViewIntegration.kt} | 19 ++++++++- .../MaliciousSiteBlockerWebViewIntegration.kt | 39 ------------------- 11 files changed, 26 insertions(+), 49 deletions(-) rename app/src/main/java/com/duckduckgo/app/browser/webview/{RealMaliciousSiteBlockerWebViewIntegration.kt => MaliciousSiteBlockerWebViewIntegration.kt} (92%) delete mode 100644 browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index ac56d97e6e1a..edafca37b598 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -59,6 +59,7 @@ import com.duckduckgo.app.browser.pageloadpixel.firstpaint.PagePaintedHandler import com.duckduckgo.app.browser.print.PrintInjector import com.duckduckgo.app.browser.trafficquality.AndroidFeaturesHeaderPlugin import com.duckduckgo.app.browser.uriloaded.UriLoadedManager +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.global.model.Site import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.app.statistics.pixels.Pixel @@ -66,7 +67,6 @@ import com.duckduckgo.autoconsent.api.Autoconsent import com.duckduckgo.autofill.api.BrowserAutofill import com.duckduckgo.autofill.api.InternalTestUserChecker import com.duckduckgo.browser.api.JsInjectorPlugin -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.browser.api.WebViewVersionProvider import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.utils.CurrentTimeProvider diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt index ad115fcf07c6..3ffb8f747d0e 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt @@ -42,7 +42,6 @@ import com.duckduckgo.app.trackerdetection.TrackerDetector import com.duckduckgo.app.trackerdetection.model.TrackerStatus import com.duckduckgo.app.trackerdetection.model.TrackerType import com.duckduckgo.app.trackerdetection.model.TrackingEvent -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.feature.toggles.api.FeatureToggle diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt index fd521dfdf4af..c8092cab9873 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt @@ -24,7 +24,7 @@ import com.duckduckgo.app.browser.* import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.cookies.api.CookieManagerProvider import kotlinx.coroutines.test.TestScope diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt index 50f945aa1b23..baf7d6ce4b7f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt @@ -28,6 +28,7 @@ import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.adclick.api.AdClickManager import com.duckduckgo.app.browser.WebViewRequestInterceptor import com.duckduckgo.app.browser.useragent.provideUserAgentOverridePluginPoint +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.fakes.FeatureToggleFake import com.duckduckgo.app.fakes.UserAgentFake import com.duckduckgo.app.fakes.UserAllowListRepositoryFake @@ -52,7 +53,6 @@ import com.duckduckgo.app.trackerdetection.db.TdsCnameEntityDao import com.duckduckgo.app.trackerdetection.db.TdsDomainEntityDao import com.duckduckgo.app.trackerdetection.db.TdsEntityDao import com.duckduckgo.app.trackerdetection.db.WebTrackersBlockedDao -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.test.FileUtilities import com.duckduckgo.duckplayer.api.DuckPlayer diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt index 8d6e3015751f..92114b7d3152 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt @@ -27,6 +27,7 @@ import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.adclick.api.AdClickManager import com.duckduckgo.app.browser.WebViewRequestInterceptor import com.duckduckgo.app.browser.useragent.provideUserAgentOverridePluginPoint +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.fakes.FeatureToggleFake import com.duckduckgo.app.fakes.UserAgentFake import com.duckduckgo.app.fakes.UserAllowListRepositoryFake @@ -50,7 +51,6 @@ import com.duckduckgo.app.trackerdetection.db.TdsCnameEntityDao import com.duckduckgo.app.trackerdetection.db.TdsDomainEntityDao import com.duckduckgo.app.trackerdetection.db.TdsEntityDao import com.duckduckgo.app.trackerdetection.db.WebTrackersBlockedDao -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.test.FileUtilities import com.duckduckgo.duckplayer.api.DuckPlayer diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index e97175e533b1..8a83de393f1d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -60,13 +60,13 @@ import com.duckduckgo.app.browser.pageloadpixel.firstpaint.PagePaintedHandler import com.duckduckgo.app.browser.print.PrintInjector import com.duckduckgo.app.browser.trafficquality.AndroidFeaturesHeaderPlugin import com.duckduckgo.app.browser.uriloaded.UriLoadedManager +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.autoconsent.api.Autoconsent import com.duckduckgo.autofill.api.BrowserAutofill import com.duckduckgo.autofill.api.InternalTestUserChecker import com.duckduckgo.browser.api.JsInjectorPlugin -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.CurrentTimeProvider import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.plugins.PluginPoint diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 0adaad641815..7838e6ddaa15 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -22,6 +22,7 @@ import android.webkit.WebResourceResponse import android.webkit.WebView import androidx.annotation.WorkerThread import com.duckduckgo.adclick.api.AdClickManager +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.privacy.db.PrivacyProtectionCountDao import com.duckduckgo.app.privacy.model.TrustedSites import com.duckduckgo.app.surrogates.ResourceSurrogates @@ -29,7 +30,6 @@ import com.duckduckgo.app.trackerdetection.CloakedCnameDetector import com.duckduckgo.app.trackerdetection.TrackerDetector import com.duckduckgo.app.trackerdetection.model.TrackerStatus import com.duckduckgo.app.trackerdetection.model.TrackingEvent -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.AppUrl import com.duckduckgo.common.utils.DefaultDispatcherProvider import com.duckduckgo.common.utils.DispatcherProvider diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index b0a80118f83f..8cbb11ed5dfb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -55,6 +55,7 @@ import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister import com.duckduckgo.app.browser.urlextraction.DOMUrlExtractor import com.duckduckgo.app.browser.urlextraction.JsUrlExtractor import com.duckduckgo.app.browser.urlextraction.UrlExtractingWebViewClient +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.fire.* import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository @@ -72,7 +73,6 @@ import com.duckduckgo.app.surrogates.ResourceSurrogates import com.duckduckgo.app.tabs.ui.GridViewColumnCalculator import com.duckduckgo.app.trackerdetection.CloakedCnameDetector import com.duckduckgo.app.trackerdetection.TrackerDetector -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.cookies.api.CookieManagerProvider import com.duckduckgo.cookies.api.ThirdPartyCookieNames diff --git a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt index 63516ae4e464..5a774ea93970 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt @@ -28,7 +28,7 @@ import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationSt import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.cookies.api.CookieManagerProvider import kotlinx.coroutines.* diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt similarity index 92% rename from app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt rename to app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index b621492409ab..782905743d66 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt @@ -24,7 +24,6 @@ import androidx.core.net.toUri import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.di.IsMainProcess import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature -import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection @@ -37,6 +36,24 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import timber.log.Timber +interface MaliciousSiteBlockerWebViewIntegration { + + suspend fun shouldIntercept( + request: WebResourceRequest, + documentUri: Uri?, + onSiteBlockedAsync: () -> Unit, + ): WebResourceResponse? + + suspend fun shouldOverrideUrlLoading( + url: Uri, + webViewUrl: Uri?, + isForMainFrame: Boolean, + onSiteBlockedAsync: () -> Unit, + ): Boolean + + fun onPageLoadStarted() +} + @ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) @ContributesBinding(AppScope::class, MaliciousSiteBlockerWebViewIntegration::class) class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( diff --git a/browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt b/browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt deleted file mode 100644 index 4b9a2e8c0b75..000000000000 --- a/browser-api/src/main/java/com/duckduckgo/browser/api/MaliciousSiteBlockerWebViewIntegration.kt +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright (c) 2024 DuckDuckGo - * - * 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. - */ - -package com.duckduckgo.browser.api - -import android.net.Uri -import android.webkit.WebResourceRequest -import android.webkit.WebResourceResponse - -interface MaliciousSiteBlockerWebViewIntegration { - - suspend fun shouldIntercept( - request: WebResourceRequest, - documentUri: Uri?, - onSiteBlockedAsync: () -> Unit, - ): WebResourceResponse? - - suspend fun shouldOverrideUrlLoading( - url: Uri, - webViewUrl: Uri?, - isForMainFrame: Boolean, - onSiteBlockedAsync: () -> Unit, - ): Boolean - - fun onPageLoadStarted() -} From 73dd3b946ddf50f089f25834b57fd1f5a44ee2db Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Wed, 11 Dec 2024 13:10:25 +0100 Subject: [PATCH 08/12] Modify MaliciousSiteProtection API --- .../app/browser/WebViewRequestInterceptorTest.kt | 1 + .../app/browser/BrowserWebViewClient.kt | 4 ++-- .../app/browser/WebViewRequestInterceptor.kt | 4 ++-- .../MaliciousSiteBlockerWebViewIntegration.kt | 15 ++++++++------- ...lMaliciousSiteBlockerWebViewIntegrationTest.kt | 15 ++++++++------- .../api/MaliciousSiteProtection.kt | 8 +++++++- .../impl/RealMaliciousSiteProtection.kt | 5 +++-- 7 files changed, 31 insertions(+), 21 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt index 3ffb8f747d0e..0c6a45bfa6ff 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt @@ -29,6 +29,7 @@ import androidx.core.net.toUri import androidx.test.annotation.UiThreadTest import com.duckduckgo.adclick.api.AdClickManager import com.duckduckgo.app.browser.useragent.provideUserAgentOverridePluginPoint +import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.fakes.FeatureToggleFake import com.duckduckgo.app.fakes.UserAgentFake import com.duckduckgo.app.fakes.UserAllowListRepositoryFake diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 8a83de393f1d..afe946f4ef0d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -128,7 +128,7 @@ class BrowserWebViewClient @Inject constructor( private var shouldOpenDuckPlayerInNewTab: Boolean = true - private val onSiteBlockedAsync: () -> Unit = { + private val confirmationCallback: (isMalicious: Boolean) -> Unit = { // TODO (cbarreiro): Handle site blocked asynchronously } @@ -170,7 +170,7 @@ class BrowserWebViewClient @Inject constructor( url, webView.url?.toUri(), isForMainFrame, - onSiteBlockedAsync, + confirmationCallback, ) } ) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 7838e6ddaa15..4c34e29a671d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -99,11 +99,11 @@ class WebViewRequestInterceptor( ): WebResourceResponse? { val url: Uri? = request.url - val onSiteBlockedAsync: () -> Unit = { + val confirmationCallback: (isMalicious: Boolean) -> Unit = { // TODO (cbarreiro): Handle site blocked asynchronously } - maliciousSiteProtectionWebViewIntegration.shouldIntercept(request, documentUri, onSiteBlockedAsync)?.let { + maliciousSiteProtectionWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let { // TODO (cbarreiro): Handle site blocked synchronously return it } diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index 782905743d66..7b109c8e2a4d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt @@ -27,6 +27,7 @@ import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMaliciousResult.MALICIOUS import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding @@ -41,14 +42,14 @@ interface MaliciousSiteBlockerWebViewIntegration { suspend fun shouldIntercept( request: WebResourceRequest, documentUri: Uri?, - onSiteBlockedAsync: () -> Unit, + confirmationCallback: (isMalicious: Boolean) -> Unit, ): WebResourceResponse? suspend fun shouldOverrideUrlLoading( url: Uri, webViewUrl: Uri?, isForMainFrame: Boolean, - onSiteBlockedAsync: () -> Unit, + confirmationCallback: (isMalicious: Boolean) -> Unit, ): Boolean fun onPageLoadStarted() @@ -87,7 +88,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( override suspend fun shouldIntercept( request: WebResourceRequest, documentUri: Uri?, - onSiteBlockedAsync: () -> Unit, + confirmationCallback: (isMalicious: Boolean) -> Unit, ): WebResourceResponse? { if (!isFeatureEnabled) { return null @@ -109,12 +110,12 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( } if (request.isForMainFrame && decodedUrl.toUri() == documentUri) { - if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { return WebResourceResponse(null, null, null) } processedUrls.add(decodedUrl) } else if (isForIframe(request) && documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host) { - if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { return WebResourceResponse(null, null, null) } processedUrls.add(decodedUrl) @@ -126,7 +127,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( url: Uri, webViewUrl: Uri?, isForMainFrame: Boolean, - onSiteBlockedAsync: () -> Unit, + confirmationCallback: (isMalicious: Boolean) -> Unit, ): Boolean { if (!isFeatureEnabled) { return false @@ -140,7 +141,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( } if (isForMainFrame && decodedUrl.toUri() == webViewUrl) { - if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { return true } processedUrls.add(decodedUrl) diff --git a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt index 3ac09713ff25..b72120e1941f 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt @@ -8,6 +8,7 @@ import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory import com.duckduckgo.feature.toggles.api.Toggle.State import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMaliciousResult.MALICIOUS import junit.framework.TestCase.assertTrue import kotlinx.coroutines.test.runTest import org.junit.Assert.assertFalse @@ -75,7 +76,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { val request = mock(WebResourceRequest::class.java) whenever(request.url).thenReturn(maliciousUri) whenever(request.isForMainFrame).thenReturn(true) - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldIntercept(request, maliciousUri) {} assertNotNull(result) @@ -87,7 +88,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { whenever(request.url).thenReturn(maliciousUri) whenever(request.isForMainFrame).thenReturn(true) whenever(request.requestHeaders).thenReturn(mapOf("Sec-Fetch-Dest" to "iframe")) - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldIntercept(request, maliciousUri) {} assertNotNull(result) @@ -98,7 +99,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { val request = mock(WebResourceRequest::class.java) whenever(request.url).thenReturn(maliciousUri) whenever(request.isForMainFrame).thenReturn(false) - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldIntercept(request, maliciousUri) {} assertNull(result) @@ -106,7 +107,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { @Test fun `shouldOverride returns false when feature is enabled, is malicious, and is not mainframe`() = runTest { - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} assertFalse(result) @@ -114,7 +115,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { @Test fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe`() = runTest { - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, true) {} assertTrue(result) @@ -122,7 +123,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { @Test fun `shouldOverride returns false when feature is enabled, is malicious, and not mainframe nor iframe`() = runTest { - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} assertFalse(result) @@ -130,7 +131,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { @Test fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe but webView has different host`() = runTest { - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true) + whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) val result = testee.shouldOverrideUrlLoading(maliciousUri, exampleUri, true) {} assertFalse(result) diff --git a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt index 0e4eb8d56ddc..7f5ad066d72d 100644 --- a/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-api/src/main/kotlin/com/duckduckgo/malicioussiteprotection/api/MaliciousSiteProtection.kt @@ -20,5 +20,11 @@ import android.net.Uri interface MaliciousSiteProtection { - suspend fun isMalicious(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean + suspend fun isMalicious(url: Uri, confirmationCallback: (isMalicious: Boolean) -> Unit): IsMaliciousResult + + enum class IsMaliciousResult { + MALICIOUS, + SAFE, + WAIT_FOR_CONFIRMATION, + } } diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index c703412445d0..fb56a653a48e 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -22,6 +22,7 @@ import com.duckduckgo.app.di.IsMainProcess import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection +import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMaliciousResult import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding @@ -67,9 +68,9 @@ class RealMaliciousSiteProtection @Inject constructor( } } - override suspend fun isMalicious(url: Uri, onSiteBlockedAsync: () -> Unit): Boolean { + override suspend fun isMalicious(url: Uri, confirmationCallback: (isMalicious: Boolean) -> Unit): IsMaliciousResult { Timber.tag("MaliciousSiteProtection").d("isMalicious $url") // TODO (cbarreiro): Implement the logic to check if the URL is malicious - return false + return IsMaliciousResult.SAFE } } From 05af0440f8be08c63da914df62019434089656f3 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 16 Dec 2024 11:18:02 +0100 Subject: [PATCH 09/12] Do not compare webview URL for mainframe requests --- .../browser/webview/MaliciousSiteBlockerWebViewIntegration.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index 7b109c8e2a4d..e7b08b4edfab 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt @@ -109,7 +109,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( return null } - if (request.isForMainFrame && decodedUrl.toUri() == documentUri) { + if (request.isForMainFrame) { if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { return WebResourceResponse(null, null, null) } From dd7e18555cb53e13aa08b7e1c853e3a9a9f9daf5 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 16 Dec 2024 11:28:25 +0100 Subject: [PATCH 10/12] Address PR comments --- .../app/browser/BrowserWebViewClientTest.kt | 4 +- .../browser/WebViewRequestInterceptorTest.kt | 36 +---------------- .../referencetests/DomainsReferenceTest.kt | 2 +- .../referencetests/SurrogatesReferenceTest.kt | 2 +- .../app/browser/BrowserWebViewClient.kt | 14 +++---- .../app/browser/WebViewRequestInterceptor.kt | 5 +-- .../app/browser/di/BrowserModule.kt | 2 + .../UrlExtractingWebViewClient.kt | 1 - .../MaliciousSiteBlockerWebViewIntegration.kt | 40 ++++++++++--------- ...iciousSiteBlockerWebViewIntegrationTest.kt | 18 +++------ 10 files changed, 41 insertions(+), 83 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index edafca37b598..76d90012132e 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -199,7 +199,7 @@ class BrowserWebViewClientTest { whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0) whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1") whenever(deviceInfo.appVersion).thenReturn("1") - whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any(), any())).thenReturn(false) + whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any())).thenReturn(false) } @UiThreadTest @@ -338,7 +338,7 @@ class BrowserWebViewClientTest { TestScope().launch { val webResourceRequest = mock() testee.shouldInterceptRequest(webView, webResourceRequest) - verify(requestInterceptor).shouldIntercept(any(), any(), any(), any(), any()) + verify(requestInterceptor).shouldIntercept(any(), any(), any(), any()) } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt index 0c6a45bfa6ff..c1aca6bcaf18 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt @@ -119,6 +119,7 @@ class WebViewRequestInterceptorTest { cloakedCnameDetector = mockCloakedCnameDetector, requestFilterer = mockRequestFilterer, duckPlayer = mockDuckPlayer, + maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection, ) } @@ -130,7 +131,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) assertCancelledResponse(response) @@ -143,7 +143,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -157,7 +156,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -172,7 +170,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -187,7 +184,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) verify(mockHttpsUpgrader).upgrade(any()) @@ -203,7 +199,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -217,7 +212,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) verify(mockDuckPlayer).intercept(any(), any(), any()) @@ -231,7 +225,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -245,7 +238,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -259,7 +251,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) assertRequestCanContinueToLoad(response) @@ -272,7 +263,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duckduckgo.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -286,7 +276,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "donttrack.us/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -300,7 +289,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "spreadprivacy.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -314,7 +302,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duckduckhack.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -328,7 +315,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "privatebrowsingmyths.com/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -342,7 +328,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "duck.co/a/b/c?q=123".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -359,7 +344,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockListener, ) @@ -377,7 +361,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockListener, ) @@ -395,7 +378,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -418,7 +400,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -442,7 +423,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -457,7 +437,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -474,7 +453,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -492,7 +470,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -510,7 +487,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -528,7 +504,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -546,7 +521,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -563,7 +537,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -580,7 +553,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -597,7 +569,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -614,7 +585,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = null, webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = mockWebViewClientListener, ) @@ -674,7 +644,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -713,7 +682,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -735,7 +703,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) @@ -755,7 +722,6 @@ class WebViewRequestInterceptorTest { request = mockRequest, documentUri = "foo.com".toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt index baf7d6ce4b7f..f98a8e43cce8 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt @@ -176,6 +176,7 @@ class DomainsReferenceTest(private val testCase: TestCase) { adClickManager = mockAdClickManager, cloakedCnameDetector = CloakedCnameDetectorImpl(tdsCnameEntityDao, mockTrackerAllowlist, mockUserAllowListRepository), requestFilterer = mockRequestFilterer, + maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection, duckPlayer = mockDuckPlayer, ) } @@ -200,7 +201,6 @@ class DomainsReferenceTest(private val testCase: TestCase) { request = mockRequest, documentUri = testCase.siteURL.toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt index 92114b7d3152..98673c5f285a 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt @@ -172,6 +172,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) { adClickManager = mockAdClickManager, cloakedCnameDetector = mockCloakedCnameDetector, requestFilterer = mockRequestFilterer, + maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection, duckPlayer = mockDuckPlayer, ) } @@ -189,7 +190,6 @@ class SurrogatesReferenceTest(private val testCase: TestCase) { request = mockRequest, documentUri = testCase.siteURL.toUri(), webView = webView, - maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection, webViewClientListener = null, ) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index afe946f4ef0d..03196139f267 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -165,14 +165,11 @@ class BrowserWebViewClient @Inject constructor( Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url") webViewClientListener?.onShouldOverride() - if (runBlocking { - maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading( - url, - webView.url?.toUri(), - isForMainFrame, - confirmationCallback, - ) - } + if (maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading( + url, + isForMainFrame, + confirmationCallback, + ) ) { // TODO (cbarreiro): Handle site blocked synchronously return true @@ -534,7 +531,6 @@ class BrowserWebViewClient @Inject constructor( request, webView, documentUrl?.toUri(), - maliciousSiteProtectionWebViewIntegration, webViewClientListener, ) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 4c34e29a671d..1c0aabd3615c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -49,7 +49,6 @@ interface RequestInterceptor { request: WebResourceRequest, webView: WebView, documentUri: Uri?, - maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, webViewClientListener: WebViewClientListener?, ): WebResourceResponse? @@ -73,6 +72,7 @@ class WebViewRequestInterceptor( private val cloakedCnameDetector: CloakedCnameDetector, private val requestFilterer: RequestFilterer, private val duckPlayer: DuckPlayer, + private val maliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, private val dispatchers: DispatcherProvider = DefaultDispatcherProvider(), ) : RequestInterceptor { @@ -94,7 +94,6 @@ class WebViewRequestInterceptor( request: WebResourceRequest, webView: WebView, documentUri: Uri?, - maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, webViewClientListener: WebViewClientListener?, ): WebResourceResponse? { val url: Uri? = request.url @@ -103,7 +102,7 @@ class WebViewRequestInterceptor( // TODO (cbarreiro): Handle site blocked asynchronously } - maliciousSiteProtectionWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let { + maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let { // TODO (cbarreiro): Handle site blocked synchronously return it } diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index 8cbb11ed5dfb..a89f37d3565a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -204,6 +204,7 @@ class BrowserModule { cloakedCnameDetector: CloakedCnameDetector, requestFilterer: RequestFilterer, duckPlayer: DuckPlayer, + maliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, ): RequestInterceptor = WebViewRequestInterceptor( resourceSurrogates, @@ -216,6 +217,7 @@ class BrowserModule { cloakedCnameDetector, requestFilterer, duckPlayer, + maliciousSiteBlockerWebViewIntegration, ) @Provides diff --git a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt index 5a774ea93970..f92cfa71e904 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt @@ -84,7 +84,6 @@ class UrlExtractingWebViewClient( request, webView, documentUrl, - maliciousSiteProtection, null, ) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index e7b08b4edfab..329dbf449f2f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt @@ -31,10 +31,12 @@ import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMali import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding +import dagger.SingleInstanceIn import java.net.URLDecoder import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import timber.log.Timber interface MaliciousSiteBlockerWebViewIntegration { @@ -45,9 +47,8 @@ interface MaliciousSiteBlockerWebViewIntegration { confirmationCallback: (isMalicious: Boolean) -> Unit, ): WebResourceResponse? - suspend fun shouldOverrideUrlLoading( + fun shouldOverrideUrlLoading( url: Uri, - webViewUrl: Uri?, isForMainFrame: Boolean, confirmationCallback: (isMalicious: Boolean) -> Unit, ): Boolean @@ -55,6 +56,7 @@ interface MaliciousSiteBlockerWebViewIntegration { fun onPageLoadStarted() } +@SingleInstanceIn(AppScope::class) @ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) @ContributesBinding(AppScope::class, MaliciousSiteBlockerWebViewIntegration::class) class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( @@ -123,30 +125,32 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( return null } - override suspend fun shouldOverrideUrlLoading( + override fun shouldOverrideUrlLoading( url: Uri, - webViewUrl: Uri?, isForMainFrame: Boolean, confirmationCallback: (isMalicious: Boolean) -> Unit, ): Boolean { - if (!isFeatureEnabled) { - return false - } - val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() + return runBlocking { + if (!isFeatureEnabled) { + return@runBlocking false + } + val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() - if (processedUrls.contains(decodedUrl)) { - processedUrls.remove(decodedUrl) - Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") - return false - } + if (processedUrls.contains(decodedUrl)) { + processedUrls.remove(decodedUrl) + Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") + return@runBlocking false + } - if (isForMainFrame && decodedUrl.toUri() == webViewUrl) { - if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { - return true + // iframes always go through the shouldIntercept method, so we only need to check the main frame here + if (isForMainFrame) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { + return@runBlocking true + } + processedUrls.add(decodedUrl) } - processedUrls.add(decodedUrl) + false } - return false } private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" || diff --git a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt index b72120e1941f..b4141551c217 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt @@ -49,7 +49,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest { updateFeatureEnabled(false) - val result = testee.shouldOverrideUrlLoading(exampleUri, null, true) {} + val result = testee.shouldOverrideUrlLoading(exampleUri, true) {} assertFalse(result) } @@ -67,7 +67,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverrideUrlLoading returns false when url is already processed`() = runTest { testee.processedUrls.add(exampleUri.toString()) - val result = testee.shouldOverrideUrlLoading(exampleUri, exampleUri, true) {} + val result = testee.shouldOverrideUrlLoading(exampleUri, true) {} assertFalse(result) } @@ -109,7 +109,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverride returns false when feature is enabled, is malicious, and is not mainframe`() = runTest { whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} + val result = testee.shouldOverrideUrlLoading(maliciousUri, false) {} assertFalse(result) } @@ -117,7 +117,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe`() = runTest { whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, true) {} + val result = testee.shouldOverrideUrlLoading(maliciousUri, true) {} assertTrue(result) } @@ -125,15 +125,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverride returns false when feature is enabled, is malicious, and not mainframe nor iframe`() = runTest { whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} - assertFalse(result) - } - - @Test - fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe but webView has different host`() = runTest { - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - - val result = testee.shouldOverrideUrlLoading(maliciousUri, exampleUri, true) {} + val result = testee.shouldOverrideUrlLoading(maliciousUri, false) {} assertFalse(result) } From f51bf51d62d6901e25e17e7fd29bd8bbc017b3aa Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Tue, 17 Dec 2024 10:10:56 +0100 Subject: [PATCH 11/12] Extract RC caching to a different class --- .../impl/MaliciousSiteProtectionRCFeature.kt | 85 +++++++++++++++++++ .../impl/RealMaliciousSiteProtection.kt | 43 +--------- 2 files changed, 87 insertions(+), 41 deletions(-) create mode 100644 malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSiteProtectionRCFeature.kt diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSiteProtectionRCFeature.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSiteProtectionRCFeature.kt new file mode 100644 index 000000000000..16db750c413d --- /dev/null +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSiteProtectionRCFeature.kt @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * 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. + */ + +package com.duckduckgo.malicioussiteprotection.impl + +import com.duckduckgo.app.di.AppCoroutineScope +import com.duckduckgo.app.di.IsMainProcess +import com.duckduckgo.common.utils.DispatcherProvider +import com.duckduckgo.di.scopes.AppScope +import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin +import com.squareup.anvil.annotations.ContributesBinding +import com.squareup.anvil.annotations.ContributesMultibinding +import dagger.SingleInstanceIn +import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch +import org.json.JSONObject + +interface MaliciousSiteProtectionRCFeature { + fun isFeatureEnabled(): Boolean + fun getHashPrefixUpdateFrequency(): Long + fun getFilterSetUpdateFrequency(): Long +} + +@SingleInstanceIn(AppScope::class) +@ContributesBinding(AppScope::class, MaliciousSiteProtectionRCFeature::class) +@ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) +class RealMaliciousSiteProtectionRCFeature @Inject constructor( + private val dispatchers: DispatcherProvider, + private val maliciousSiteProtectionFeature: MaliciousSiteProtectionFeature, + @IsMainProcess private val isMainProcess: Boolean, + @AppCoroutineScope private val appCoroutineScope: CoroutineScope, +) : MaliciousSiteProtectionRCFeature, PrivacyConfigCallbackPlugin { + private var isFeatureEnabled = false + + private var hashPrefixUpdateFrequency = 20L + private var filterSetUpdateFrequency = 720L + + init { + if (isMainProcess) { + loadToMemory() + } + } + + override fun onPrivacyConfigDownloaded() { + loadToMemory() + } + + override fun getFilterSetUpdateFrequency(): Long { + return filterSetUpdateFrequency + } + + override fun getHashPrefixUpdateFrequency(): Long { + return hashPrefixUpdateFrequency + } + + override fun isFeatureEnabled(): Boolean { + return isFeatureEnabled + } + + private fun loadToMemory() { + appCoroutineScope.launch(dispatchers.io()) { + isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() + maliciousSiteProtectionFeature.self().getSettings()?.let { + JSONObject(it).let { settings -> + hashPrefixUpdateFrequency = settings.getLong("hashPrefixUpdateFrequency") + filterSetUpdateFrequency = settings.getLong("filterSetUpdateFrequency") + } + } + } + } +} diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt index fb56a653a48e..7ff0dd28d5d5 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/RealMaliciousSiteProtection.kt @@ -17,56 +17,17 @@ package com.duckduckgo.malicioussiteprotection.impl import android.net.Uri -import com.duckduckgo.app.di.AppCoroutineScope -import com.duckduckgo.app.di.IsMainProcess -import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMaliciousResult -import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding -import com.squareup.anvil.annotations.ContributesMultibinding import javax.inject.Inject -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch -import org.json.JSONObject import timber.log.Timber @ContributesBinding(AppScope::class, MaliciousSiteProtection::class) -@ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) class RealMaliciousSiteProtection @Inject constructor( - private val dispatchers: DispatcherProvider, - private val maliciousSiteProtectionFeature: MaliciousSiteProtectionFeature, - @IsMainProcess private val isMainProcess: Boolean, - @AppCoroutineScope private val appCoroutineScope: CoroutineScope, -) : MaliciousSiteProtection, PrivacyConfigCallbackPlugin { - - private var isFeatureEnabled = false - - private var hashPrefixUpdateFrequency = 20L - private var filterSetUpdateFrequency = 720L - - init { - if (isMainProcess) { - loadToMemory() - } - } - - override fun onPrivacyConfigDownloaded() { - loadToMemory() - } - - private fun loadToMemory() { - appCoroutineScope.launch(dispatchers.io()) { - isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() - maliciousSiteProtectionFeature.self().getSettings()?.let { - JSONObject(it).let { settings -> - hashPrefixUpdateFrequency = settings.getLong("hashPrefixUpdateFrequency") - filterSetUpdateFrequency = settings.getLong("filterSetUpdateFrequency") - } - } - } - } + maliciousSiteProtectionRCFeature: MaliciousSiteProtectionRCFeature, +) : MaliciousSiteProtection { override suspend fun isMalicious(url: Uri, confirmationCallback: (isMalicious: Boolean) -> Unit): IsMaliciousResult { Timber.tag("MaliciousSiteProtection").d("isMalicious $url") From 7f2f516ec865cc9f0fde6e02ef8e07185d2fe7a5 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Thu, 19 Dec 2024 10:55:18 +0100 Subject: [PATCH 12/12] Extract part of shouldOverrideUrlLoading to RequestInterceptor --- .../app/browser/BrowserWebViewClientTest.kt | 1 - .../UrlExtractingWebViewClientTest.kt | 1 - .../app/browser/BrowserWebViewClient.kt | 14 ++------ .../app/browser/WebViewRequestInterceptor.kt | 35 +++++++++++++++---- .../app/browser/di/BrowserModule.kt | 2 -- .../UrlExtractingWebViewClient.kt | 2 -- .../MaliciousSiteBlockerWebViewIntegration.kt | 2 -- .../AndroidBrowserConfigFeature.kt | 2 -- 8 files changed, 31 insertions(+), 28 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 76d90012132e..3061968b86f1 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -191,7 +191,6 @@ class BrowserWebViewClientTest { mockDuckDuckGoUrlDetector, mockUriLoadedManager, mockAndroidFeaturesHeaderPlugin, - mockMaliciousSiteProtection, ) testee.webViewClientListener = listener whenever(webResourceRequest.url).thenReturn(Uri.EMPTY) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt index c8092cab9873..df4281b4a21b 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt @@ -65,7 +65,6 @@ class UrlExtractingWebViewClientTest { TestScope(), coroutinesTestRule.testDispatcherProvider, urlExtractor, - mockMaliciousSiteProtection, ) whenever(cookieManagerProvider.get()).thenReturn(cookieManager) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 03196139f267..a0cb6de7cda9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -60,7 +60,6 @@ import com.duckduckgo.app.browser.pageloadpixel.firstpaint.PagePaintedHandler import com.duckduckgo.app.browser.print.PrintInjector import com.duckduckgo.app.browser.trafficquality.AndroidFeaturesHeaderPlugin import com.duckduckgo.app.browser.uriloaded.UriLoadedManager -import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.autoconsent.api.Autoconsent @@ -118,7 +117,6 @@ class BrowserWebViewClient @Inject constructor( private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector, private val uriLoadedManager: UriLoadedManager, private val androidFeaturesHeaderPlugin: AndroidFeaturesHeaderPlugin, - private val maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration, ) : WebViewClient() { var webViewClientListener: WebViewClientListener? = null @@ -164,14 +162,7 @@ class BrowserWebViewClient @Inject constructor( try { Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url") webViewClientListener?.onShouldOverride() - - if (maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading( - url, - isForMainFrame, - confirmationCallback, - ) - ) { - // TODO (cbarreiro): Handle site blocked synchronously + if (requestInterceptor.shouldOverrideUrlLoading(url, isForMainFrame)) { return true } @@ -424,12 +415,11 @@ class BrowserWebViewClient @Inject constructor( // See https://app.asana.com/0/0/1206159443951489/f (WebView limitations) if (it != ABOUT_BLANK && start == null) { start = currentTimeProvider.elapsedRealtime() - maliciousSiteProtectionWebViewIntegration.onPageLoadStarted() + requestInterceptor.onPageStarted(url) } handleMediaPlayback(webView, it) autoconsent.injectAutoconsent(webView, url) adClickManager.detectAdDomain(url) - requestInterceptor.onPageStarted(url) appCoroutineScope.launch(dispatcherProvider.io()) { thirdPartyCookieManager.processUriForThirdPartyCookies(webView, url.toUri()) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 1c0aabd3615c..5c3ec224118e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -59,6 +59,12 @@ interface RequestInterceptor { ): WebResourceResponse? fun onPageStarted(url: String) + + @WorkerThread + fun shouldOverrideUrlLoading( + url: Uri, + isForMainFrame: Boolean, + ): Boolean } class WebViewRequestInterceptor( @@ -78,6 +84,7 @@ class WebViewRequestInterceptor( override fun onPageStarted(url: String) { requestFilterer.registerOnPageCreated(url) + maliciousSiteBlockerWebViewIntegration.onPageLoadStarted() } /** @@ -98,12 +105,10 @@ class WebViewRequestInterceptor( ): WebResourceResponse? { val url: Uri? = request.url - val confirmationCallback: (isMalicious: Boolean) -> Unit = { - // TODO (cbarreiro): Handle site blocked asynchronously - } - - maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let { - // TODO (cbarreiro): Handle site blocked synchronously + maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) { + handleSiteBlocked() + }?.let { + handleSiteBlocked() return it } @@ -172,6 +177,24 @@ class WebViewRequestInterceptor( return getWebResourceResponse(request, documentUrl, null) } + override fun shouldOverrideUrlLoading(url: Uri, isForMainFrame: Boolean): Boolean { + if (maliciousSiteBlockerWebViewIntegration.shouldOverrideUrlLoading( + url, + isForMainFrame, + ) { + handleSiteBlocked() + } + ) { + handleSiteBlocked() + return true + } + return false + } + + private fun handleSiteBlocked() { + // TODO (cbarreiro): Handle site blocked + } + private fun getWebResourceResponse( request: WebResourceRequest, documentUrl: Uri, diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index a89f37d3565a..ca07b2c52796 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -122,7 +122,6 @@ class BrowserModule { @AppCoroutineScope appCoroutineScope: CoroutineScope, dispatcherProvider: DispatcherProvider, urlExtractor: DOMUrlExtractor, - maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration, ): UrlExtractingWebViewClient { return UrlExtractingWebViewClient( webViewHttpAuthStore, @@ -133,7 +132,6 @@ class BrowserModule { appCoroutineScope, dispatcherProvider, urlExtractor, - maliciousSiteProtection, ) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt index f92cfa71e904..f8c4e2a509cc 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt @@ -28,7 +28,6 @@ import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationSt import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore -import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.cookies.api.CookieManagerProvider import kotlinx.coroutines.* @@ -43,7 +42,6 @@ class UrlExtractingWebViewClient( private val appCoroutineScope: CoroutineScope, private val dispatcherProvider: DispatcherProvider, private val urlExtractor: DOMUrlExtractor, - private val maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration, ) : WebViewClient() { var urlExtractionListener: UrlExtractionListener? = null diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index 329dbf449f2f..ab00b5d78f2a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt @@ -31,7 +31,6 @@ import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMali import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding -import dagger.SingleInstanceIn import java.net.URLDecoder import javax.inject.Inject import kotlinx.coroutines.CoroutineScope @@ -56,7 +55,6 @@ interface MaliciousSiteBlockerWebViewIntegration { fun onPageLoadStarted() } -@SingleInstanceIn(AppScope::class) @ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class) @ContributesBinding(AppScope::class, MaliciousSiteBlockerWebViewIntegration::class) class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( diff --git a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt index 593d6cc2b9d0..c3354e110fcb 100644 --- a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt +++ b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt @@ -19,7 +19,6 @@ package com.duckduckgo.app.pixels.remoteconfig import com.duckduckgo.anvil.annotations.ContributesRemoteFeature import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.feature.toggles.api.Toggle -import com.duckduckgo.feature.toggles.api.Toggle.InternalAlwaysEnabled /** * This is the class that represents the browser feature flags @@ -98,7 +97,6 @@ interface AndroidBrowserConfigFeature { * sub-feature flag enabled * If the remote feature is not present defaults to `false` */ - @InternalAlwaysEnabled @Toggle.DefaultValue(false) fun enableMaliciousSiteProtection(): Toggle }