Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add localStorage exceptions to duckduckgo.com #5338

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,29 @@ package com.duckduckgo.app.browser

import android.annotation.SuppressLint
import android.content.Context
import android.webkit.ValueCallback
import android.webkit.WebStorage
import android.webkit.WebStorage.Origin
import android.webkit.WebView
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore
import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage
import com.duckduckgo.app.global.file.FileDeleter
import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature
import com.duckduckgo.cookies.api.DuckDuckGoCookieManager
import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.withContext
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.mockito.kotlin.any
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever

@Suppress("RemoveExplicitTypeArguments")
@SuppressLint("NoHardcodedCoroutineDispatcher")
Expand All @@ -42,7 +51,24 @@ class WebViewDataManagerTest {
private val context = InstrumentationRegistry.getInstrumentation().targetContext
private val mockFileDeleter: FileDeleter = mock()
private val mockWebViewHttpAuthStore: WebViewHttpAuthStore = mock()
private val testee = WebViewDataManager(context, WebViewSessionInMemoryStorage(), mockCookieManager, mockFileDeleter, mockWebViewHttpAuthStore)
private val feature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java)
private val testee = WebViewDataManager(
context,
WebViewSessionInMemoryStorage(),
mockCookieManager,
mockFileDeleter,
mockWebViewHttpAuthStore,
feature,
)

@Before
fun setup() {
doAnswer { invocation ->
val callback = invocation.arguments[0] as ValueCallback<Map<String, Origin>>
callback.onReceiveValue(emptyMap()) // Simulate callback invocation
null
}.whenever(mockStorage).getOrigins(any())
}

@Test
fun whenDataClearedThenWebViewHistoryCleared() = runTest {
Expand Down Expand Up @@ -76,7 +102,8 @@ class WebViewDataManagerTest {
withContext(Dispatchers.Main) {
val webView = TestWebView(context)
testee.clearData(webView, mockStorage)
verify(mockStorage).deleteAllData()
// we call deleteOrigin() instead and we should make sure we don't call deleteAllData()
verify(mockStorage, never()).deleteAllData()
}
}

Expand Down
68 changes: 47 additions & 21 deletions app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@ package com.duckduckgo.app.browser

import android.content.Context
import android.webkit.WebStorage
import android.webkit.WebStorage.Origin
import android.webkit.WebView
import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore
import com.duckduckgo.app.browser.session.WebViewSessionStorage
import com.duckduckgo.app.global.file.FileDeleter
import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature
import com.duckduckgo.cookies.api.DuckDuckGoCookieManager
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
import java.io.File
import javax.inject.Inject
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine
import timber.log.Timber

interface WebDataManager {
suspend fun clearData(
Expand All @@ -35,12 +43,15 @@ interface WebDataManager {
fun clearWebViewSessions()
}

@ContributesBinding(AppScope::class)
@SingleInstanceIn(AppScope::class)
class WebViewDataManager @Inject constructor(
private val context: Context,
private val webViewSessionStorage: WebViewSessionStorage,
private val cookieManager: DuckDuckGoCookieManager,
private val fileDeleter: FileDeleter,
private val webViewHttpAuthStore: WebViewHttpAuthStore,
private val androidBrowserConfigFeature: AndroidBrowserConfigFeature,
) : WebDataManager {

override suspend fun clearData(
Expand All @@ -53,7 +64,7 @@ class WebViewDataManager @Inject constructor(
clearFormData(webView)
clearAuthentication(webView)
clearExternalCookies()
clearWebViewDirectories(exclusions = WEBVIEW_FILES_EXCLUDED_FROM_DELETION)
clearWebViewDirectories()
}

private fun clearWebViewCache(webView: WebView) {
Expand All @@ -64,26 +75,52 @@ class WebViewDataManager @Inject constructor(
webView.clearHistory()
}

private fun clearWebStorage(webStorage: WebStorage) {
webStorage.deleteAllData()
private suspend fun clearWebStorage(webStorage: WebStorage) {
suspendCoroutine { continuation ->
webStorage.getOrigins { origins ->
kotlin.runCatching {
for (origin in origins.values) {
val originString = (origin as Origin).origin

// Check if this is the domain to exclude
if (!originString.endsWith(".duckduckgo.com")) {
aitorvs marked this conversation as resolved.
Show resolved Hide resolved
// Delete all other origins
Timber.d("aitor delete $originString / $origin")
webStorage.deleteOrigin(originString)
}
}
continuation.resume(Unit)
}.onFailure {
// fallback, if we crash we delete everything
webStorage.deleteAllData()
continuation.resume(Unit)
}
}
}
}

private fun clearFormData(webView: WebView) {
webView.clearFormData()
}

/**
* Deletes web view directory content. The Cookies file is kept as we clear cookies separately to avoid a crash and maintain ddg cookies.
* Cookies may appear in files:
* app_webview/Cookies
* app_webview/Default/Cookies
* Deletes web view directory content except the following directories
* app_webview/Cookies
* app_webview/Default/Cookies
* app_webview/Default/Local Storage
*
* the excluded directories above are to avoid clearing unnecessary cookies and because localStorage is cleared using clearWebStorage
*/
private suspend fun clearWebViewDirectories(exclusions: List<String>) {
private suspend fun clearWebViewDirectories() {
val dataDir = context.applicationInfo.dataDir
fileDeleter.deleteContents(File(dataDir, WEBVIEW_DATA_DIRECTORY_NAME), exclusions)
fileDeleter.deleteContents(File(dataDir, "app_webview"), listOf("Default", "Cookies"))

// We don't delete the Default dir as Cookies may be inside however we do clear any other content
fileDeleter.deleteContents(File(dataDir, WEBVIEW_DEFAULT_DIRECTORY_NAME), exclusions)
if (androidBrowserConfigFeature.deleteLocalStorageKillSwitch().isEnabled()) {
fileDeleter.deleteContents(File(dataDir, "app_webview/Default"), listOf("Cookies"))
} else {
fileDeleter.deleteContents(File(dataDir, "app_webview/Default"), listOf("Cookies", "Local Storage"))
}
}

private suspend fun clearAuthentication(webView: WebView) {
Expand All @@ -98,15 +135,4 @@ class WebViewDataManager @Inject constructor(
override fun clearWebViewSessions() {
webViewSessionStorage.deleteAllSessions()
}

companion object {
private const val WEBVIEW_DATA_DIRECTORY_NAME = "app_webview"
private const val WEBVIEW_DEFAULT_DIRECTORY_NAME = "app_webview/Default"
private const val DATABASES_DIRECTORY_NAME = "databases"

private val WEBVIEW_FILES_EXCLUDED_FROM_DELETION = listOf(
"Default",
"Cookies",
)
}
}
12 changes: 0 additions & 12 deletions app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ import com.duckduckgo.app.trackerdetection.CloakedCnameDetector
import com.duckduckgo.app.trackerdetection.TrackerDetector
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.cookies.api.CookieManagerProvider
import com.duckduckgo.cookies.api.DuckDuckGoCookieManager
import com.duckduckgo.cookies.api.ThirdPartyCookieNames
import com.duckduckgo.customtabs.api.CustomTabDetector
import com.duckduckgo.di.scopes.AppScope
Expand Down Expand Up @@ -159,17 +158,6 @@ class BrowserModule {
@Provides
fun webViewSessionStorage(): WebViewSessionStorage = WebViewSessionInMemoryStorage()

@SingleInstanceIn(AppScope::class)
@Provides
fun webDataManager(
context: Context,
webViewSessionStorage: WebViewSessionStorage,
cookieManager: DuckDuckGoCookieManager,
fileDeleter: FileDeleter,
webViewHttpAuthStore: WebViewHttpAuthStore,
): WebDataManager =
WebViewDataManager(context, webViewSessionStorage, cookieManager, fileDeleter, webViewHttpAuthStore)

@Provides
fun clipboardManager(context: Context): ClipboardManager {
return context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,11 @@ interface AndroidBrowserConfigFeature {
*/
@Toggle.DefaultValue(false)
fun featuresRequestHeader(): Toggle

/**
* When enabled we should delete the app_webview/Default/Local Storage folder. If all goes well, we should not need to set this to `true`
* in remote config
*/
@Toggle.DefaultValue(false)
fun deleteLocalStorageKillSwitch(): Toggle
}
Loading