From 3cff846ef833b6c4bcae09ac8f61022bf00fdd0a Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Wed, 5 Feb 2025 18:52:36 +0100 Subject: [PATCH] Add algorithm pixels --- .../MaliciousSiteBlockerWebViewIntegration.kt | 23 ++++++++++++-- .../com/duckduckgo/app/pixels/AppPixelName.kt | 2 ++ ...iciousSiteBlockerWebViewIntegrationTest.kt | 9 ++++-- .../build.gradle | 2 ++ .../impl/MaliciousSitePixelName.kt | 23 ++++++++++++++ .../impl/data/MaliciousSiteRepository.kt | 30 ++++++++++++------- .../data/RealMaliciousSiteRepositoryTest.kt | 23 +++++++++++++- 7 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSitePixelName.kt 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 bc166e120648..9e4c16788ef6 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 @@ -24,8 +24,10 @@ import com.duckduckgo.app.browser.webview.ExemptedUrlsHolder.ExemptedUrl import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.di.IsMainProcess +import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.app.settings.db.SettingsDataStore +import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection @@ -97,6 +99,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( @AppCoroutineScope private val appCoroutineScope: CoroutineScope, private val exemptedUrlsHolder: ExemptedUrlsHolder, @IsMainProcess private val isMainProcess: Boolean, + private val pixel: Pixel, ) : MaliciousSiteBlockerWebViewIntegration, PrivacyConfigCallbackPlugin { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @@ -161,19 +164,31 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( } val belongsToCurrentPage = documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host - if (request.isForMainFrame || (isForIframe(request) && belongsToCurrentPage)) { - when (val result = checkMaliciousUrl(decodedUrl, confirmationCallback)) { + val isForIframe = isForIframe(request) && belongsToCurrentPage + if (request.isForMainFrame || isForIframe) { + val result = checkMaliciousUrl(decodedUrl) { + if (isForIframe && it is Malicious) { + firePixelForMaliciousIframe(it.feed) + } + confirmationCallback(it) + } + when (result) { is ConfirmedResult -> { when (val status = result.status) { is Malicious -> { + if (isForIframe) { + firePixelForMaliciousIframe(status.feed) + } return IsMaliciousViewData.MaliciousSite(url, status.feed, false) } + is Safe -> { processedUrls.add(decodedUrl) return IsMaliciousViewData.Safe } } } + is WaitForConfirmation -> { processedUrls.add(decodedUrl) return IsMaliciousViewData.WaitForConfirmation @@ -231,6 +246,10 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( } } + private fun firePixelForMaliciousIframe(feed: Feed) { + pixel.fire(AppPixelName.MALICIOUS_SITE_DETECTED_IN_IFRAME, mapOf("category" to feed.name.lowercase())) + } + private suspend fun checkMaliciousUrl( url: String, confirmationCallback: (maliciousStatus: MaliciousStatus) -> Unit, diff --git a/app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt b/app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt index a7d53ccdeb5b..235d089160d9 100644 --- a/app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt +++ b/app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt @@ -397,4 +397,6 @@ enum class AppPixelName(override val pixelName: String) : Pixel.PixelName { SET_AS_DEFAULT_PROMPT_CLICK("m_set-as-default_prompt_click"), SET_AS_DEFAULT_PROMPT_DISMISSED("m_set-as-default_prompt_dismissed"), SET_AS_DEFAULT_IN_MENU_CLICK("m_set-as-default_in-menu_click"), + + MALICIOUS_SITE_DETECTED_IN_IFRAME("m_malicious-site-protection_iframe-loaded"), } 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 0842ac86129b..ba5bb8d98519 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 @@ -6,8 +6,10 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import com.duckduckgo.app.browser.webview.ExemptedUrlsHolder.ExemptedUrl import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.MaliciousSite import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.Safe +import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.app.settings.db.SettingsDataStore +import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory import com.duckduckgo.feature.toggles.api.Toggle.State @@ -41,6 +43,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { private val maliciousSiteProtection: MaliciousSiteProtection = mock(MaliciousSiteProtection::class.java) private val mockSettingsDataStore: SettingsDataStore = mock(SettingsDataStore::class.java) private val mockExemptedUrlsHolder = mock(ExemptedUrlsHolder::class.java) + private val mockPixel: Pixel = mock() private val fakeAndroidBrowserConfigFeature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java) private val maliciousUri = "http://malicious.com".toUri() private val exampleUri = "http://example.com".toUri() @@ -52,6 +55,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { appCoroutineScope = coroutineRule.testScope, isMainProcess = true, exemptedUrlsHolder = mockExemptedUrlsHolder, + pixel = mockPixel, ) @Before @@ -120,11 +124,12 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldInterceptRequest returns result when feature is enabled, setting 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(request.isForMainFrame).thenReturn(false) + whenever(request.requestHeaders).thenReturn(mapOf("Sec-Fetch-Dest" to "iframe", "Referer" to maliciousUri.toString())) whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(ConfirmedResult(Malicious(MALWARE))) val result = testee.shouldIntercept(request, maliciousUri) {} + verify(mockPixel).fire(AppPixelName.MALICIOUS_SITE_DETECTED_IN_IFRAME, mapOf("category" to MALWARE.name.lowercase())) assertEquals(MaliciousSite(maliciousUri, MALWARE, false), result) } diff --git a/malicious-site-protection/malicious-site-protection-impl/build.gradle b/malicious-site-protection/malicious-site-protection-impl/build.gradle index 7077945b75fa..111cb9f53dbe 100644 --- a/malicious-site-protection/malicious-site-protection-impl/build.gradle +++ b/malicious-site-protection/malicious-site-protection-impl/build.gradle @@ -54,6 +54,8 @@ dependencies { implementation Google.android.material + implementation project(path: ':statistics-api') + testImplementation AndroidX.test.ext.junit testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.5.2' testImplementation Testing.junit4 diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSitePixelName.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSitePixelName.kt new file mode 100644 index 000000000000..af0243fa2358 --- /dev/null +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/MaliciousSitePixelName.kt @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2025 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.statistics.pixels.Pixel + +enum class MaliciousSitePixelName(override val pixelName: String) : Pixel.PixelName { + MALICIOUS_SITE_CLIENT_TIMEOUT("m_malicious-site-protection_client-timeout"), +} diff --git a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/MaliciousSiteRepository.kt b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/MaliciousSiteRepository.kt index 494e9cbf8164..dfc3f7089ce3 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/MaliciousSiteRepository.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/main/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/MaliciousSiteRepository.kt @@ -16,11 +16,13 @@ package com.duckduckgo.malicioussiteprotection.impl.data +import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed.MALWARE import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed.PHISHING +import com.duckduckgo.malicioussiteprotection.impl.MaliciousSitePixelName.MALICIOUS_SITE_CLIENT_TIMEOUT import com.duckduckgo.malicioussiteprotection.impl.data.db.MaliciousSiteDao import com.duckduckgo.malicioussiteprotection.impl.data.db.RevisionEntity import com.duckduckgo.malicioussiteprotection.impl.data.network.FilterResponse @@ -42,7 +44,9 @@ import com.duckduckgo.malicioussiteprotection.impl.models.Type.HASH_PREFIXES import com.squareup.anvil.annotations.ContributesBinding import dagger.SingleInstanceIn import javax.inject.Inject +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout interface MaliciousSiteRepository { suspend fun containsHashPrefix(hashPrefix: String): Boolean @@ -58,6 +62,7 @@ class RealMaliciousSiteRepository @Inject constructor( private val maliciousSiteDao: MaliciousSiteDao, private val maliciousSiteService: MaliciousSiteService, private val dispatcherProvider: DispatcherProvider, + private val pixels: Pixel, ) : MaliciousSiteRepository { override suspend fun containsHashPrefix(hashPrefix: String): Boolean { @@ -79,18 +84,23 @@ class RealMaliciousSiteRepository @Inject constructor( override suspend fun matches(hashPrefix: String): List { return try { - maliciousSiteService.getMatches(hashPrefix).matches.mapNotNull { - val feed = when (it.feed.uppercase()) { - PHISHING.name -> PHISHING - MALWARE.name -> MALWARE - else -> null - } - if (feed != null) { - Match(it.hostname, it.url, it.regex, it.hash, feed) - } else { - null + withTimeout(1000) { + maliciousSiteService.getMatches(hashPrefix).matches.mapNotNull { + val feed = when (it.feed.uppercase()) { + PHISHING.name -> PHISHING + MALWARE.name -> MALWARE + else -> null + } + if (feed != null) { + Match(it.hostname, it.url, it.regex, it.hash, feed) + } else { + null + } } } + } catch (e: TimeoutCancellationException) { + pixels.fire(MALICIOUS_SITE_CLIENT_TIMEOUT) + listOf() } catch (e: Exception) { listOf() } diff --git a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/RealMaliciousSiteRepositoryTest.kt b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/RealMaliciousSiteRepositoryTest.kt index f5befe477ed3..5c91baa64b01 100644 --- a/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/RealMaliciousSiteRepositoryTest.kt +++ b/malicious-site-protection/malicious-site-protection-impl/src/test/kotlin/com/duckduckgo/malicioussiteprotection/impl/data/RealMaliciousSiteRepositoryTest.kt @@ -1,6 +1,8 @@ package com.duckduckgo.malicioussiteprotection.impl.data +import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed.PHISHING +import com.duckduckgo.malicioussiteprotection.impl.MaliciousSitePixelName.MALICIOUS_SITE_CLIENT_TIMEOUT import com.duckduckgo.malicioussiteprotection.impl.data.db.FilterEntity import com.duckduckgo.malicioussiteprotection.impl.data.db.HashPrefixEntity import com.duckduckgo.malicioussiteprotection.impl.data.db.MaliciousSiteDao @@ -17,6 +19,7 @@ import com.duckduckgo.malicioussiteprotection.impl.models.FilterSetWithRevision. import com.duckduckgo.malicioussiteprotection.impl.models.HashPrefixesWithRevision.PhishingHashPrefixesWithRevision import com.duckduckgo.malicioussiteprotection.impl.models.Match import com.duckduckgo.malicioussiteprotection.impl.models.Type +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue @@ -34,7 +37,13 @@ class RealMaliciousSiteRepositoryTest { private val maliciousSiteDao: MaliciousSiteDao = mock() private val maliciousSiteService: MaliciousSiteService = mock() - private val repository = RealMaliciousSiteRepository(maliciousSiteDao, maliciousSiteService, coroutineRule.testDispatcherProvider) + private val mockPixel: Pixel = mock() + private val repository = RealMaliciousSiteRepository( + maliciousSiteDao, + maliciousSiteService, + coroutineRule.testDispatcherProvider, + mockPixel, + ) @Test fun loadFilters_updatesFiltersWhenNetworkRevisionIsHigher() = runTest { @@ -136,4 +145,16 @@ class RealMaliciousSiteRepositoryTest { assertEquals(matchesResponse.matches.map { Match(it.hostname, it.url, it.regex, it.hash, PHISHING) }, result) } + + @Test + fun matches_returnsEmptyListOnTimeout() = runTest { + val hashPrefix = "testPrefix" + + whenever(maliciousSiteService.getMatches(hashPrefix)).thenThrow(TimeoutCancellationException::class.java) + + val result = repository.matches(hashPrefix) + + assertTrue(result.isEmpty()) + verify(mockPixel).fire(MALICIOUS_SITE_CLIENT_TIMEOUT) + } }