From a2a274d48a478e9db423be4c0f384853af1da235 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Mon, 17 Jun 2019 11:42:27 +0100 Subject: [PATCH 1/2] Delete most of the app_webview directory contents upon data clearing This is partly to ensure that the cookies DB journal file is also cleared up upon data clearing, and helps ensure no sensitive browsing data is retained on the device after the data has been cleared --- .../app/browser/WebViewDataManagerTest.kt | 3 ++- .../app/fire/WebViewCookieManagerTest.kt | 13 +++++++++++- .../duckduckgo/app/browser/WebDataManager.kt | 20 +++++++++++++++++++ .../app/browser/di/BrowserModule.kt | 4 ++-- .../global/view/ClearPersonalDataAction.kt | 1 + 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt index 1609d6482496..617316129097 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt @@ -35,7 +35,8 @@ class WebViewDataManagerTest { private val mockCookieManager: DuckDuckGoCookieManager = mock() private val mockStorage: WebStorage = mock() - private val testee = WebViewDataManager(WebViewSessionInMemoryStorage(), mockCookieManager) + private val context = InstrumentationRegistry.getInstrumentation().targetContext + private val testee = WebViewDataManager(context, WebViewSessionInMemoryStorage(), mockCookieManager) @UiThreadTest @Test diff --git a/app/src/androidTest/java/com/duckduckgo/app/fire/WebViewCookieManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/fire/WebViewCookieManagerTest.kt index 158e46c7b46b..f9b01b3fce24 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/fire/WebViewCookieManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/fire/WebViewCookieManagerTest.kt @@ -24,6 +24,8 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import kotlin.coroutines.resume +import kotlin.coroutines.suspendCoroutine @Suppress("RemoveExplicitTypeArguments") class WebViewCookieManagerTest { @@ -33,10 +35,19 @@ class WebViewCookieManagerTest { private val cookieManager: CookieManager = CookieManager.getInstance() @Before - fun setup() { + fun setup() = runBlocking { + removeExistingCookies() testee = WebViewCookieManager(cookieManager, host) } + private suspend fun removeExistingCookies() { + withContext(Dispatchers.Main) { + suspendCoroutine { continuation -> + cookieManager.removeAllCookies { continuation.resume(Unit) } + } + } + } + @Test fun whenExternalCookiesClearedThenInternalCookiesRecreated() = runBlocking { cookieManager.setCookie(host, "da=abc") diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt index c41f882261c9..b5e75dc95ed2 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt @@ -16,21 +16,25 @@ package com.duckduckgo.app.browser +import android.content.Context import android.os.Build import android.webkit.WebStorage import android.webkit.WebView import android.webkit.WebViewDatabase import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.fire.DuckDuckGoCookieManager +import java.io.File import javax.inject.Inject interface WebDataManager { suspend fun clearExternalCookies() fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) fun clearWebViewSessions() + suspend fun deleteWebViewDirectory() } class WebViewDataManager @Inject constructor( + private val context: Context, private val webViewSessionStorage: WebViewSessionStorage, private val cookieManager: DuckDuckGoCookieManager ) : WebDataManager { @@ -64,6 +68,13 @@ class WebViewDataManager @Inject constructor( webView.clearFormData() } + override suspend fun deleteWebViewDirectory() { + val webViewDataDirectory = File(context.applicationInfo.dataDir, WEBVIEW_DATA_DIRECTORY_NAME) + val files = webViewDataDirectory.listFiles() ?: return + val filesToDelete = files.filterNot { FILENAMES_EXCLUDED_FROM_DELETION.contains(it.name) } + filesToDelete.forEach { it.deleteRecursively() } + } + /** * Deprecated and not needed on Oreo or later */ @@ -83,4 +94,13 @@ class WebViewDataManager @Inject constructor( override fun clearWebViewSessions() { webViewSessionStorage.deleteAllSessions() } + + companion object { + private const val WEBVIEW_DATA_DIRECTORY_NAME = "app_webview" + + private val FILENAMES_EXCLUDED_FROM_DELETION = listOf( + "Cookies", + "Local Storage" + ) + } } 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 4bdc402bad7d..c05d58847ac4 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 @@ -91,8 +91,8 @@ class BrowserModule { @Singleton @Provides - fun webDataManager(webViewSessionStorage: WebViewSessionStorage, cookieManager: DuckDuckGoCookieManager): WebDataManager = - WebViewDataManager(webViewSessionStorage, cookieManager) + fun webDataManager(context: Context, webViewSessionStorage: WebViewSessionStorage, cookieManager: DuckDuckGoCookieManager): WebDataManager = + WebViewDataManager(context, webViewSessionStorage, cookieManager) @Provides fun clipboardManager(context: Context): ClipboardManager { diff --git a/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt b/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt index 75b47f5691c0..80a544a99f40 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/ClearPersonalDataAction.kt @@ -106,6 +106,7 @@ class ClearPersonalDataAction @Inject constructor( dataManager.clearData(createWebView(), createWebStorage(), WebViewDatabase.getInstance(context)) dataManager.clearExternalCookies() + dataManager.deleteWebViewDirectory() appCacheClearer.clearCache() From 4a52afcdffc0c133e518e69423255229000906fd Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Mon, 17 Jun 2019 12:09:27 +0100 Subject: [PATCH 2/2] Exclude the android webview cache directory from being deleted We are implementing the deletion in the same way as Firefox Focus here, who don't touch this directory or its contents as they observed disk caching would be disabled altogether if you do modify it. --- .../app/browser/WebViewDataManagerTest.kt | 4 +- .../com/duckduckgo/app/di/TestAppComponent.kt | 3 +- .../duckduckgo/app/browser/WebDataManager.kt | 8 ++-- .../app/browser/di/BrowserModule.kt | 10 ++++- .../com/duckduckgo/app/di/AppComponent.kt | 3 +- .../java/com/duckduckgo/app/di/FileModule.kt | 33 ++++++++++++++ .../com/duckduckgo/app/di/PrivacyModule.kt | 5 ++- .../duckduckgo/app/fire/AppCacheClearer.kt | 21 ++++++--- .../duckduckgo/app/global/file/FileDeleter.kt | 44 +++++++++++++++++++ 9 files changed, 114 insertions(+), 17 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/di/FileModule.kt create mode 100644 app/src/main/java/com/duckduckgo/app/global/file/FileDeleter.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt index 617316129097..d6010f0a539e 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewDataManagerTest.kt @@ -24,6 +24,7 @@ import androidx.test.annotation.UiThreadTest import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.app.browser.session.WebViewSessionInMemoryStorage import com.duckduckgo.app.fire.DuckDuckGoCookieManager +import com.duckduckgo.app.global.file.FileDeleter import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import kotlinx.coroutines.runBlocking @@ -36,7 +37,8 @@ class WebViewDataManagerTest { private val mockCookieManager: DuckDuckGoCookieManager = mock() private val mockStorage: WebStorage = mock() private val context = InstrumentationRegistry.getInstrumentation().targetContext - private val testee = WebViewDataManager(context, WebViewSessionInMemoryStorage(), mockCookieManager) + private val mockFileDeleter: FileDeleter = mock() + private val testee = WebViewDataManager(context, WebViewSessionInMemoryStorage(), mockCookieManager, mockFileDeleter) @UiThreadTest @Test diff --git a/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt b/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt index 7e0994b797c9..1977915fcf14 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt @@ -65,7 +65,8 @@ import javax.inject.Singleton PrivacyModule::class, WidgetModule::class, RatingModule::class, - AppUsageModule::class + AppUsageModule::class, + FileModule::class ] ) interface TestAppComponent : AppComponent { diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt index b5e75dc95ed2..7f66df228dd7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebDataManager.kt @@ -23,6 +23,7 @@ import android.webkit.WebView import android.webkit.WebViewDatabase import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.fire.DuckDuckGoCookieManager +import com.duckduckgo.app.global.file.FileDeleter import java.io.File import javax.inject.Inject @@ -36,7 +37,8 @@ interface WebDataManager { class WebViewDataManager @Inject constructor( private val context: Context, private val webViewSessionStorage: WebViewSessionStorage, - private val cookieManager: DuckDuckGoCookieManager + private val cookieManager: DuckDuckGoCookieManager, + private val fileDeleter: FileDeleter ) : WebDataManager { override fun clearData(webView: WebView, webStorage: WebStorage, webViewDatabase: WebViewDatabase) { @@ -70,9 +72,7 @@ class WebViewDataManager @Inject constructor( override suspend fun deleteWebViewDirectory() { val webViewDataDirectory = File(context.applicationInfo.dataDir, WEBVIEW_DATA_DIRECTORY_NAME) - val files = webViewDataDirectory.listFiles() ?: return - val filesToDelete = files.filterNot { FILENAMES_EXCLUDED_FROM_DELETION.contains(it.name) } - filesToDelete.forEach { it.deleteRecursively() } + fileDeleter.deleteContents(webViewDataDirectory, FILENAMES_EXCLUDED_FROM_DELETION) } /** 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 c05d58847ac4..5b2764eff2dd 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 @@ -30,6 +30,7 @@ import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.fire.DuckDuckGoCookieManager import com.duckduckgo.app.fire.WebViewCookieManager import com.duckduckgo.app.global.AppUrl +import com.duckduckgo.app.global.file.FileDeleter import com.duckduckgo.app.global.install.AppInstallStore import com.duckduckgo.app.httpsupgrade.HttpsUpgrader import com.duckduckgo.app.privacy.db.PrivacyProtectionCountDao @@ -91,8 +92,13 @@ class BrowserModule { @Singleton @Provides - fun webDataManager(context: Context, webViewSessionStorage: WebViewSessionStorage, cookieManager: DuckDuckGoCookieManager): WebDataManager = - WebViewDataManager(context, webViewSessionStorage, cookieManager) + fun webDataManager( + context: Context, + webViewSessionStorage: WebViewSessionStorage, + cookieManager: DuckDuckGoCookieManager, + fileDeleter: FileDeleter + ): WebDataManager = + WebViewDataManager(context, webViewSessionStorage, cookieManager, fileDeleter) @Provides fun clipboardManager(context: Context): ClipboardManager { diff --git a/app/src/main/java/com/duckduckgo/app/di/AppComponent.kt b/app/src/main/java/com/duckduckgo/app/di/AppComponent.kt index 7a033610c853..0c8ec96cb57a 100644 --- a/app/src/main/java/com/duckduckgo/app/di/AppComponent.kt +++ b/app/src/main/java/com/duckduckgo/app/di/AppComponent.kt @@ -63,7 +63,8 @@ import javax.inject.Singleton PrivacyModule::class, WidgetModule::class, RatingModule::class, - AppUsageModule::class + AppUsageModule::class, + FileModule::class ] ) interface AppComponent : AndroidInjector { diff --git a/app/src/main/java/com/duckduckgo/app/di/FileModule.kt b/app/src/main/java/com/duckduckgo/app/di/FileModule.kt new file mode 100644 index 000000000000..969018763861 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/di/FileModule.kt @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2019 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.di + +import com.duckduckgo.app.global.file.AndroidFileDeleter +import com.duckduckgo.app.global.file.FileDeleter +import dagger.Module +import dagger.Provides + + +@Module +class FileModule { + + @Provides + fun providesFileDeleter(): FileDeleter { + return AndroidFileDeleter() + } + +} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt index af2ddefcb0a4..4b34a0733623 100644 --- a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt @@ -20,6 +20,7 @@ import android.content.Context import com.duckduckgo.app.browser.WebDataManager import com.duckduckgo.app.entities.EntityMapping import com.duckduckgo.app.fire.* +import com.duckduckgo.app.global.file.FileDeleter import com.duckduckgo.app.global.install.AppInstallStore import com.duckduckgo.app.global.view.ClearDataAction import com.duckduckgo.app.global.view.ClearPersonalDataAction @@ -82,7 +83,7 @@ class PrivacyModule { @Provides @Singleton - fun appCacheCleaner(context: Context): AppCacheClearer { - return AndroidAppCacheClearer(context) + fun appCacheCleaner(context: Context, fileDeleter: FileDeleter): AppCacheClearer { + return AndroidAppCacheClearer(context, fileDeleter) } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/fire/AppCacheClearer.kt b/app/src/main/java/com/duckduckgo/app/fire/AppCacheClearer.kt index 689b5473fdda..fa87294ee492 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/AppCacheClearer.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/AppCacheClearer.kt @@ -17,8 +17,7 @@ package com.duckduckgo.app.fire import android.content.Context -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext +import com.duckduckgo.app.global.file.FileDeleter interface AppCacheClearer { @@ -27,12 +26,22 @@ interface AppCacheClearer { } -class AndroidAppCacheClearer(private val context: Context) : AppCacheClearer { +class AndroidAppCacheClearer(private val context: Context, private val fileDeleter: FileDeleter) : AppCacheClearer { override suspend fun clearCache() { - withContext(Dispatchers.IO) { - context.cacheDir.deleteRecursively() - } + fileDeleter.deleteContents(context.cacheDir, FILENAMES_EXCLUDED_FROM_DELETION) + } + + companion object { + + /* Exclude this WebView cache directory, based on warning from Firefox Focus: + * "If the folder or its contents are deleted, WebView will stop using the disk cache entirely." + */ + private const val WEBVIEW_CACHE_DIR = "org.chromium.android_webview" + + private val FILENAMES_EXCLUDED_FROM_DELETION = listOf( + WEBVIEW_CACHE_DIR + ) } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/global/file/FileDeleter.kt b/app/src/main/java/com/duckduckgo/app/global/file/FileDeleter.kt new file mode 100644 index 000000000000..cc6468b05717 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/global/file/FileDeleter.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2019 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.global.file + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.io.File + + +interface FileDeleter { + + /** + * Delete the contents of the given directory, but don't delete the directory itself + * + * Optionally: specify an exclusion list. Files with names exactly matching will not be deleted. + * Note, the exclusion list only applies to the top-level directory. All files in subdirectories will be deleted, regardless of exclusion list. + */ + suspend fun deleteContents(parentDirectory: File, excludedFiles: List = emptyList()) +} + +class AndroidFileDeleter : FileDeleter { + + override suspend fun deleteContents(parentDirectory: File, excludedFiles: List) { + withContext(Dispatchers.IO) { + val files = parentDirectory.listFiles() ?: return@withContext + val filesToDelete = files.filterNot { excludedFiles.contains(it.name) } + filesToDelete.forEach { it.deleteRecursively() } + } + } +} \ No newline at end of file