From fc58aba059ebc7ecaaf56812e023c67cf284151f Mon Sep 17 00:00:00 2001
From: chris-cwa <69595386+chris-cwa@users.noreply.github.com>
Date: Thu, 19 Nov 2020 18:35:51 +0100
Subject: [PATCH] Prevent premature task abortions, Store timestamp first
(EXPOSUREAPP-3878) (#1668)
* do not prematurely abort diagnosis keys task, if there is no last server fetch time; before anything else store time stamp after successfully submitting keys to enf
* comments and logs
* We need to reset the tracked exposure detections on app reset.
* Make sure that all flow preferences reset when we use the app reset function.
* Add workaround for EncryptedSharedPreferences, it's `clear` behavior is not like a normal sharedpreference,
as no listeners are called when keys are removed.
* Fix failing test.
Co-authored-by: Matthias Urhahn <matthias.urhahn@sap.com>
---
.../download/DownloadDiagnosisKeysTask.kt | 10 ++++++----
.../download/KeyPackageSyncSettings.kt | 7 +++++++
.../DefaultExposureDetectionTracker.kt | 7 +++++++
.../ExposureDetectionTracker.kt | 2 ++
.../de/rki/coronawarnapp/storage/LocalData.kt | 4 ++++
.../java/de/rki/coronawarnapp/util/DataReset.kt | 11 ++++++++++-
.../util/preferences/FlowPreference.kt | 16 ++++++++++++++++
.../preferences/SharedPreferenceExtensions.kt | 17 +++++++++++++++++
.../util/security/SecurityHelper.kt | 3 ++-
.../preferences/MockFlowPreference.kt | 1 +
.../preferences/MockSharedPreferences.kt | 9 +++++----
11 files changed, 77 insertions(+), 10 deletions(-)
create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/SharedPreferenceExtensions.kt
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt
index 298354d97..a6719b321 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt
@@ -83,11 +83,12 @@ class DownloadDiagnosisKeysTask @Inject constructor(
if (wasLastDetectionPerformedRecently(now, exposureConfig, trackedExposureDetections)) {
// At most one detection every 6h
+ Timber.tag(TAG).i("task aborted, because detection was performed recently")
return object : Task.Result {}
}
if (hasRecentDetectionAndNoNewFiles(now, keySyncResult, trackedExposureDetections)) {
- // Last check was within 24h, and there are no new files.
+ Timber.tag(TAG).i("task aborted, last check was within 24h, and there are no new files")
return object : Task.Result {}
}
@@ -111,13 +112,14 @@ class DownloadDiagnosisKeysTask @Inject constructor(
)
Timber.tag(TAG).d("Diagnosis Keys provided (success=%s, token=%s)", isSubmissionSuccessful, token)
- internalProgress.send(Progress.ApiSubmissionFinished)
- throwIfCancelled()
-
+ // EXPOSUREAPP-3878 write timestamp immediately after submission,
+ // so that progress observers can rely on a clean app state
if (isSubmissionSuccessful) {
saveTimestamp(currentDate, rollbackItems)
}
+ internalProgress.send(Progress.ApiSubmissionFinished)
+
return object : Task.Result {}
} catch (error: Exception) {
Timber.tag(TAG).e(error)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyPackageSyncSettings.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyPackageSyncSettings.kt
index 46499c312..767788b05 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyPackageSyncSettings.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyPackageSyncSettings.kt
@@ -1,9 +1,11 @@
package de.rki.coronawarnapp.diagnosiskeys.download
+import android.annotation.SuppressLint
import android.content.Context
import com.google.gson.Gson
import de.rki.coronawarnapp.util.di.AppContext
import de.rki.coronawarnapp.util.preferences.FlowPreference
+import de.rki.coronawarnapp.util.preferences.clearAndNotify
import de.rki.coronawarnapp.util.serialization.BaseGson
import org.joda.time.Instant
import javax.inject.Inject
@@ -32,6 +34,11 @@ class KeyPackageSyncSettings @Inject constructor(
writer = FlowPreference.gsonWriter(gson)
)
+ @SuppressLint("ApplySharedPref")
+ fun clear() {
+ prefs.clearAndNotify()
+ }
+
data class LastDownload(
val startedAt: Instant,
val finishedAt: Instant? = null,
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt
index c5e4b8073..b40e1e6d0 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt
@@ -134,6 +134,13 @@ class DefaultExposureDetectionTracker @Inject constructor(
}
}
+ override fun clear() {
+ Timber.i("clear()")
+ detectionStates.updateSafely {
+ emptyMap()
+ }
+ }
+
companion object {
private const val TAG = "DefaultExposureDetectionTracker"
private const val MAX_ENTRY_SIZE = 5
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt
index 4d9bcf0c6..ee1a302ea 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt
@@ -8,4 +8,6 @@ interface ExposureDetectionTracker {
fun trackNewExposureDetection(identifier: String)
fun finishExposureDetection(identifier: String, result: TrackedExposureDetection.Result)
+
+ fun clear()
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/LocalData.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/LocalData.kt
index 2e0347c40..26a4902fc 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/LocalData.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/LocalData.kt
@@ -726,4 +726,8 @@ object LocalData {
putBoolean(PREFERENCE_INTEROPERABILITY_IS_USED_AT_LEAST_ONCE, value)
}
}
+
+ fun clear() {
+ lastTimeDiagnosisKeysFetchedFlowPref.update { 0L }
+ }
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataReset.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataReset.kt
index 86690e225..fd1064bbf 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataReset.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataReset.kt
@@ -22,8 +22,11 @@ package de.rki.coronawarnapp.util
import android.annotation.SuppressLint
import android.content.Context
import de.rki.coronawarnapp.appconfig.AppConfigProvider
+import de.rki.coronawarnapp.diagnosiskeys.download.KeyPackageSyncSettings
import de.rki.coronawarnapp.diagnosiskeys.storage.KeyCacheRepository
+import de.rki.coronawarnapp.nearby.modules.detectiontracker.ExposureDetectionTracker
import de.rki.coronawarnapp.storage.AppDatabase
+import de.rki.coronawarnapp.storage.LocalData
import de.rki.coronawarnapp.storage.RiskLevelRepository
import de.rki.coronawarnapp.storage.SubmissionRepository
import de.rki.coronawarnapp.storage.interoperability.InteroperabilityRepository
@@ -43,7 +46,9 @@ class DataReset @Inject constructor(
@AppContext private val context: Context,
private val keyCacheRepository: KeyCacheRepository,
private val appConfigProvider: AppConfigProvider,
- private val interoperabilityRepository: InteroperabilityRepository
+ private val interoperabilityRepository: InteroperabilityRepository,
+ private val exposureDetectionTracker: ExposureDetectionTracker,
+ private val keyPackageSyncSettings: KeyPackageSyncSettings
) {
private val mutex = Mutex()
@@ -56,6 +61,8 @@ class DataReset @Inject constructor(
Timber.w("CWA LOCAL DATA DELETION INITIATED.")
// Database Reset
AppDatabase.reset(context)
+ // Because LocalData does not behave like a normal shared preference
+ LocalData.clear()
// Shared Preferences Reset
SecurityHelper.resetSharedPrefs()
// Reset the current risk level stored in LiveData
@@ -65,6 +72,8 @@ class DataReset @Inject constructor(
keyCacheRepository.clear()
appConfigProvider.clear()
interoperabilityRepository.clear()
+ exposureDetectionTracker.clear()
+ keyPackageSyncSettings.clear()
Timber.w("CWA LOCAL DATA DELETION COMPLETED.")
}
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/FlowPreference.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/FlowPreference.kt
index c64cbedfd..2e9735937 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/FlowPreference.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/FlowPreference.kt
@@ -6,6 +6,7 @@ import com.google.gson.Gson
import de.rki.coronawarnapp.util.serialization.fromJson
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
+import timber.log.Timber
class FlowPreference<T> constructor(
private val preferences: SharedPreferences,
@@ -17,6 +18,21 @@ class FlowPreference<T> constructor(
private val flowInternal = MutableStateFlow(internalValue)
val flow: Flow<T> = flowInternal
+ private val preferenceChangeListener =
+ SharedPreferences.OnSharedPreferenceChangeListener { changedPrefs, changedKey ->
+ if (changedKey != key) return@OnSharedPreferenceChangeListener
+
+ val newValue = reader(changedPrefs, changedKey)
+ val currentvalue = flowInternal.value
+ if (currentvalue != newValue && flowInternal.compareAndSet(currentvalue, newValue)) {
+ Timber.v("%s:%s changed to %s", changedPrefs, changedKey, newValue)
+ }
+ }
+
+ init {
+ preferences.registerOnSharedPreferenceChangeListener(preferenceChangeListener)
+ }
+
private var internalValue: T
get() = reader(preferences, key)
set(newValue) {
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/SharedPreferenceExtensions.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/SharedPreferenceExtensions.kt
new file mode 100644
index 000000000..0469b3840
--- /dev/null
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/preferences/SharedPreferenceExtensions.kt
@@ -0,0 +1,17 @@
+package de.rki.coronawarnapp.util.preferences
+
+import android.content.SharedPreferences
+import androidx.core.content.edit
+import timber.log.Timber
+
+fun SharedPreferences.clearAndNotify() {
+ val currentKeys = this.all.keys.toSet()
+ Timber.v("%s clearAndNotify(): %s", this, currentKeys)
+ edit {
+ currentKeys.forEach { remove(it) }
+ }
+ // Clear does not notify anyone using registerOnSharedPreferenceChangeListener
+ edit(commit = true) {
+ clear()
+ }
+}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt
index 370ab58fd..c1c145e49 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt
@@ -27,6 +27,7 @@ import androidx.annotation.VisibleForTesting
import de.rki.coronawarnapp.exception.CwaSecurityException
import de.rki.coronawarnapp.util.di.AppInjector
import de.rki.coronawarnapp.util.di.ApplicationComponent
+import de.rki.coronawarnapp.util.preferences.clearAndNotify
import de.rki.coronawarnapp.util.security.SecurityConstants.CWA_APP_SQLITE_DB_PW
import de.rki.coronawarnapp.util.security.SecurityConstants.DB_PASSWORD_MAX_LENGTH
import de.rki.coronawarnapp.util.security.SecurityConstants.DB_PASSWORD_MIN_LENGTH
@@ -80,7 +81,7 @@ object SecurityHelper {
@SuppressLint("ApplySharedPref")
fun resetSharedPrefs() {
- globalEncryptedSharedPreferencesInstance.edit().clear().commit()
+ globalEncryptedSharedPreferencesInstance.clearAndNotify()
}
private fun getStoredDbPassword(): ByteArray? =
diff --git a/Corona-Warn-App/src/test/java/testhelpers/preferences/MockFlowPreference.kt b/Corona-Warn-App/src/test/java/testhelpers/preferences/MockFlowPreference.kt
index 33d46578e..0b855e1b9 100644
--- a/Corona-Warn-App/src/test/java/testhelpers/preferences/MockFlowPreference.kt
+++ b/Corona-Warn-App/src/test/java/testhelpers/preferences/MockFlowPreference.kt
@@ -16,5 +16,6 @@ fun <T> mockFlowPreference(
val updateCall = arg<(T) -> T>(0)
flow.value = updateCall(flow.value)
}
+
return instance
}
diff --git a/Corona-Warn-App/src/test/java/testhelpers/preferences/MockSharedPreferences.kt b/Corona-Warn-App/src/test/java/testhelpers/preferences/MockSharedPreferences.kt
index a6294b00f..c094e560e 100644
--- a/Corona-Warn-App/src/test/java/testhelpers/preferences/MockSharedPreferences.kt
+++ b/Corona-Warn-App/src/test/java/testhelpers/preferences/MockSharedPreferences.kt
@@ -3,6 +3,7 @@ package testhelpers.preferences
import android.content.SharedPreferences
class MockSharedPreferences : SharedPreferences {
+ private val listeners = mutableListOf<SharedPreferences.OnSharedPreferenceChangeListener>()
private val dataMap = mutableMapOf<String, Any>()
val dataMapPeek: Map<String, Any>
get() = dataMap.toMap()
@@ -36,12 +37,12 @@ class MockSharedPreferences : SharedPreferences {
dataMap.putAll(newData)
}
- override fun registerOnSharedPreferenceChangeListener(listener: SharedPreferences.OnSharedPreferenceChangeListener?) {
- throw NotImplementedError()
+ override fun registerOnSharedPreferenceChangeListener(listener: SharedPreferences.OnSharedPreferenceChangeListener) {
+ listeners.add(listener)
}
- override fun unregisterOnSharedPreferenceChangeListener(listener: SharedPreferences.OnSharedPreferenceChangeListener?) {
- throw NotImplementedError()
+ override fun unregisterOnSharedPreferenceChangeListener(listener: SharedPreferences.OnSharedPreferenceChangeListener) {
+ listeners.remove(listener)
}
private fun createEditor(
--
GitLab