Skip to content

Commit

Permalink
Remove duplicated code from WebViewRequestInterceptor
Browse files Browse the repository at this point in the history
  • Loading branch information
CrisBarreiro committed Jan 30, 2025
1 parent 6ea4903 commit b6d3050
Show file tree
Hide file tree
Showing 20 changed files with 322 additions and 143 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import com.duckduckgo.app.browser.trafficquality.CustomHeaderAllowedChecker
import com.duckduckgo.app.browser.trafficquality.remote.AndroidFeaturesHeaderProvider
import com.duckduckgo.app.browser.uriloaded.UriLoadedManager
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.Safe
import com.duckduckgo.app.global.model.Site
import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature
import com.duckduckgo.app.statistics.pixels.Pixel
Expand Down Expand Up @@ -211,7 +212,7 @@ class BrowserWebViewClientTest {
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0)
whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1")
whenever(deviceInfo.appVersion).thenReturn("1")
whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any())).thenReturn(false)
whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any())).thenReturn(Safe)
}

@UiThreadTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ 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.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData
import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.Safe
import com.duckduckgo.app.fakes.FeatureToggleFake
import com.duckduckgo.app.fakes.UserAgentFake
import com.duckduckgo.app.fakes.UserAllowListRepositoryFake
Expand All @@ -47,6 +49,8 @@ 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.Feed
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.MaliciousStatus
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
Expand Down Expand Up @@ -98,7 +102,7 @@ class WebViewRequestInterceptorTest {
fakeToggle,
fakeUserAllowListRepository,
)
private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock()
private val mockMaliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration = FakeMaliciousSiteBlockerWebViewIntegration()

private var webView: WebView = mock()

Expand All @@ -119,7 +123,7 @@ class WebViewRequestInterceptorTest {
cloakedCnameDetector = mockCloakedCnameDetector,
requestFilterer = mockRequestFilterer,
duckPlayer = mockDuckPlayer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteBlockerWebViewIntegration,
)
}

Expand Down Expand Up @@ -887,4 +891,33 @@ class WebViewRequestInterceptorTest {
override fun saveAtb(atb: Atb) {}
override fun clearAtb() {}
}

class FakeMaliciousSiteBlockerWebViewIntegration : MaliciousSiteBlockerWebViewIntegration {
override suspend fun shouldIntercept(
request: WebResourceRequest,
documentUri: Uri?,
confirmationCallback: (maliciousStatus: MaliciousStatus) -> Unit,
): IsMaliciousViewData {
return Safe
}

override fun shouldOverrideUrlLoading(
url: Uri,
isForMainFrame: Boolean,
confirmationCallback: (maliciousStatus: MaliciousStatus) -> Unit,
): IsMaliciousViewData {
return Safe
}

override fun onPageLoadStarted() {
// no-op
}

override fun onSiteExempted(
url: Uri,
feed: Feed,
) {
TODO("Not yet implemented")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ import com.duckduckgo.js.messaging.api.JsCallbackData
import com.duckduckgo.js.messaging.api.JsMessageCallback
import com.duckduckgo.js.messaging.api.JsMessaging
import com.duckduckgo.js.messaging.api.SubscriptionEventData
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed
import com.duckduckgo.mobile.android.app.tracking.ui.AppTrackingProtectionScreens.AppTrackerOnboardingActivityWithEmptyParamsParams
import com.duckduckgo.navigation.api.GlobalActivityStarter
import com.duckduckgo.navigation.api.GlobalActivityStarter.DeeplinkActivityParams
Expand Down Expand Up @@ -1341,7 +1342,7 @@ class BrowserTabFragment :
errorView.errorLayout.show()
}

private fun showMaliciousWarning(url: Uri) {
private fun showMaliciousWarning(url: Uri, feed: Feed) {
webViewContainer.gone()
newBrowserTab.newTabLayout.gone()
newBrowserTab.newTabContainerLayout.gone()
Expand All @@ -1352,8 +1353,8 @@ class BrowserTabFragment :
webView?.onPause()
webView?.hide()
webView?.stopLoading()
maliciousWarningView.bind { action ->
viewModel.onMaliciousSiteUserAction(action, url)
maliciousWarningView.bind(feed) { action ->
viewModel.onMaliciousSiteUserAction(action, url, feed)
}
maliciousWarningView.show()
binding.focusDummy.requestFocus()
Expand Down Expand Up @@ -1729,7 +1730,7 @@ class BrowserTabFragment :
)

is Command.WebViewError -> showError(it.errorType, it.url)
is Command.ShowWarningMaliciousSite -> showMaliciousWarning(it.url)
is Command.ShowWarningMaliciousSite -> showMaliciousWarning(it.url, it.feed)
is Command.HideWarningMaliciousSite -> hideMaliciousWarning()
is Command.EscapeMaliciousSite -> onEscapeMaliciousSite()
is Command.BypassMaliciousSiteWarning -> onBypassMaliciousWarning(it.url)
Expand Down
37 changes: 24 additions & 13 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ASK
import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler
import com.duckduckgo.app.global.events.db.UserEventKey
import com.duckduckgo.app.global.events.db.UserEventsStore
import com.duckduckgo.app.global.model.MaliciousSiteStatus
import com.duckduckgo.app.global.model.PrivacyShield
import com.duckduckgo.app.global.model.Site
import com.duckduckgo.app.global.model.SiteFactory
Expand Down Expand Up @@ -294,6 +295,9 @@ import com.duckduckgo.duckplayer.api.DuckPlayer
import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.ENABLED
import com.duckduckgo.history.api.NavigationHistory
import com.duckduckgo.js.messaging.api.JsCallbackData
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.newtabpage.impl.pixels.NewTabPixels
import com.duckduckgo.privacy.config.api.AmpLinkInfo
import com.duckduckgo.privacy.config.api.AmpLinks
Expand Down Expand Up @@ -1862,6 +1866,7 @@ class BrowserTabViewModel @Inject constructor(
fun onMaliciousSiteUserAction(
action: MaliciousSiteBlockedWarningLayout.Action,
url: Uri,
feed: Feed,
) {
when (action) {
LeaveSite -> {
Expand All @@ -1874,7 +1879,7 @@ class BrowserTabViewModel @Inject constructor(
browserShowing = true,
showPrivacyShield = HighlightableButton.Visible(enabled = true),
)
addExemptedMaliciousUrlToMemory(url)
addExemptedMaliciousUrlToMemory(url, feed)
}
}
}
Expand Down Expand Up @@ -3153,17 +3158,23 @@ class BrowserTabViewModel @Inject constructor(
command.postValue(WebViewError(errorType, url))
}

override fun onReceivedMaliciousSiteWarning(url: Uri) {
override fun onReceivedMaliciousSiteWarning(url: Uri, feed: Feed, exempted: Boolean) {
// TODO (cbarreiro): Fire pixel
loadingViewState.postValue(currentLoadingViewState().copy(isLoading = false, progress = 100, url = url.toString()))
browserViewState.postValue(
currentBrowserViewState().copy(
browserShowing = false,
showPrivacyShield = HighlightableButton.Visible(enabled = false),
maliciousSiteDetected = true,
),
)
command.postValue(ShowWarningMaliciousSite(url))
site?.maliciousSiteStatus = when (feed) {
MALWARE -> MaliciousSiteStatus.MALWARE
PHISHING -> MaliciousSiteStatus.PHISHING
}
if (!exempted) {
loadingViewState.postValue(currentLoadingViewState().copy(isLoading = false, url = url.toString()))
browserViewState.postValue(
currentBrowserViewState().copy(
browserShowing = false,
showPrivacyShield = HighlightableButton.Visible(enabled = false),
maliciousSiteDetected = true,
),
)
command.postValue(ShowWarningMaliciousSite(url, feed))
}
}

override fun recordErrorCode(
Expand Down Expand Up @@ -3756,8 +3767,8 @@ class BrowserTabViewModel @Inject constructor(
command.value = SetOnboardingDialogBackground(getBackgroundResource(lightModeEnabled))
}

fun addExemptedMaliciousUrlToMemory(url: Uri) {
maliciousSiteBlockerWebViewIntegration.onSiteExempted(url)
fun addExemptedMaliciousUrlToMemory(url: Uri, feed: Feed) {
maliciousSiteBlockerWebViewIntegration.onSiteExempted(url, feed)
}

private fun getBackgroundResource(lightModeEnabled: Boolean): Int {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
import com.duckduckgo.app.global.model.Site
import com.duckduckgo.app.surrogates.SurrogateResponse
import com.duckduckgo.app.trackerdetection.model.TrackingEvent
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed
import com.duckduckgo.site.permissions.api.SitePermissionsManager.SitePermissions

interface WebViewClientListener {
Expand Down Expand Up @@ -95,7 +96,7 @@ interface WebViewClientListener {
fun linkOpenedInNewTab(): Boolean
fun isActiveTab(): Boolean
fun onReceivedError(errorType: WebViewErrorResponse, url: String)
fun onReceivedMaliciousSiteWarning(url: Uri)
fun onReceivedMaliciousSiteWarning(url: Uri, feed: Feed, exempted: Boolean)
fun recordErrorCode(error: String, url: String)
fun recordHttpErrorCode(statusCode: Int, url: String)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ 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.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData
import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.MaliciousSite
import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.Safe
import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData.WaitForConfirmation
import com.duckduckgo.app.privacy.db.PrivacyProtectionCountDao
import com.duckduckgo.app.privacy.model.TrustedSites
import com.duckduckgo.app.surrogates.ResourceSurrogates
Expand All @@ -36,6 +40,9 @@ 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.Feed
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.MaliciousStatus
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.MaliciousStatus.Malicious
import com.duckduckgo.privacy.config.api.Gpc
import com.duckduckgo.request.filterer.api.RequestFilterer
import com.duckduckgo.user.agent.api.UserAgentProvider
Expand Down Expand Up @@ -107,12 +114,9 @@ class WebViewRequestInterceptor(
val url: Uri? = request.url

maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) { isMalicious ->
if (isMalicious) {
handleSiteBlocked(webViewClientListener, url)
}
}?.let {
handleSiteBlocked(webViewClientListener, url)
return it
handleConfirmationCallback(isMalicious, webViewClientListener, url)
}.let {
if (shouldBlock(it, webViewClientListener, url)) return WebResourceResponse(null, null, null)
}

if (requestFilterer.shouldFilterOutRequest(request, documentUri.toString())) return WebResourceResponse(null, null, null)
Expand Down Expand Up @@ -166,6 +170,17 @@ class WebViewRequestInterceptor(
return getWebResourceResponse(request, documentUri, webViewClientListener)
}

override fun shouldOverrideUrlLoading(webViewClientListener: WebViewClientListener?, url: Uri, isForMainFrame: Boolean): Boolean {
maliciousSiteBlockerWebViewIntegration.shouldOverrideUrlLoading(
url,
isForMainFrame,
) { isMalicious ->
handleConfirmationCallback(isMalicious, webViewClientListener, url)
}.let {
return shouldBlock(it, webViewClientListener, url)
}
}

override suspend fun shouldInterceptFromServiceWorker(
request: WebResourceRequest?,
documentUrl: Uri?,
Expand All @@ -180,24 +195,32 @@ class WebViewRequestInterceptor(
return getWebResourceResponse(request, documentUrl, null)
}

override fun shouldOverrideUrlLoading(webViewClientListener: WebViewClientListener?, url: Uri, isForMainFrame: Boolean): Boolean {
if (maliciousSiteBlockerWebViewIntegration.shouldOverrideUrlLoading(
url,
isForMainFrame,
) { isMalicious ->
if (isMalicious) {
handleSiteBlocked(webViewClientListener, url)
}
private fun shouldBlock(
result: IsMaliciousViewData,
webViewClientListener: WebViewClientListener?,
url: Uri?,
): Boolean {
when (result) {
WaitForConfirmation, Safe -> return false
is MaliciousSite -> {
handleSiteBlocked(webViewClientListener, url, result.feed, result.exempted)
return !result.exempted
}
) {
handleSiteBlocked(webViewClientListener, url)
return true
}
return false
}

private fun handleSiteBlocked(webViewClientListener: WebViewClientListener?, url: Uri?) {
url?.let { webViewClientListener?.onReceivedMaliciousSiteWarning(it) }
private fun handleConfirmationCallback(
isMalicious: MaliciousStatus,
webViewClientListener: WebViewClientListener?,
url: Uri?,
) {
if (isMalicious is Malicious) {
handleSiteBlocked(webViewClientListener, url, isMalicious.feed, false)
}
}

private fun handleSiteBlocked(webViewClientListener: WebViewClientListener?, url: Uri?, feed: Feed, exempted: Boolean) {
url?.let { webViewClientListener?.onReceivedMaliciousSiteWarning(it, feed, exempted) }
}

private fun getWebResourceResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.browser.api.brokensite.BrokenSiteData
import com.duckduckgo.js.messaging.api.JsCallbackData
import com.duckduckgo.js.messaging.api.SubscriptionEventData
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed
import com.duckduckgo.privacy.dashboard.api.ui.DashboardOpener
import com.duckduckgo.savedsites.api.models.SavedSite
import com.duckduckgo.site.permissions.api.SitePermissionsManager.SitePermissions
Expand Down Expand Up @@ -219,6 +220,7 @@ sealed class Command {

data class ShowWarningMaliciousSite(
val url: Uri,
val feed: Feed,
) : Command()

data object HideWarningMaliciousSite : Command()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import com.duckduckgo.app.browser.webview.MaliciousSiteBlockedWarningLayout.Acti
import com.duckduckgo.common.ui.view.gone
import com.duckduckgo.common.ui.view.show
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed.MALWARE
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.Feed.PHISHING

class MaliciousSiteBlockedWarningLayout @JvmOverloads constructor(
context: Context,
Expand All @@ -43,14 +46,13 @@ class MaliciousSiteBlockedWarningLayout @JvmOverloads constructor(
private val binding: ViewMaliciousSiteBlockedWarningBinding by viewBinding()

fun bind(
feed: Feed,
actionHandler: (Action) -> Unit,
) {
resetViewState()

with(binding) {
formatCopy()
setListeners(actionHandler)
}
formatCopy(feed)
setListeners(actionHandler)
}

private fun resetViewState() {
Expand All @@ -60,9 +62,17 @@ class MaliciousSiteBlockedWarningLayout @JvmOverloads constructor(
}
}

private fun formatCopy() {
private fun formatCopy(feed: Feed) {
with(binding) {
errorHeadline.text = HtmlCompat.fromHtml(context.getString(R.string.maliciousSiteMalwareHeadline), HtmlCompat.FROM_HTML_MODE_LEGACY)
val errorResource = when (feed) {
MALWARE -> {
R.string.maliciousSiteMalwareHeadline
}
PHISHING -> {
R.string.maliciousSitePhishingHeadline
}
}
errorHeadline.text = HtmlCompat.fromHtml(context.getString(errorResource), HtmlCompat.FROM_HTML_MODE_LEGACY)
expandedHeadline.text = HtmlCompat.fromHtml(context.getString(R.string.maliciousSiteExpandedHeadline), HtmlCompat.FROM_HTML_MODE_LEGACY)
expandedCTA.text = HtmlCompat.fromHtml(context.getString(R.string.maliciousSiteExpandedCTA), HtmlCompat.FROM_HTML_MODE_LEGACY)
}
Expand Down
Loading

0 comments on commit b6d3050

Please sign in to comment.