From 1061ee2125dc668b9b3767edb521a441b8838ab9 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Wed, 11 Nov 2020 15:40:30 +0100
Subject: [PATCH] Override EXPECT_NEW_PACKAGES if we revoked key packages
(EXPOSUREAPP-3455) (#1572)
* If we revoked any cached keys we need to force an index lookup.
+ Additional tests to check that forced index lookup is decided on correctly.
* Fix merge regressions in tests.
---
.../download/BaseKeyPackageSyncTool.kt | 22 ++++++---
.../download/DayPackageSyncTool.kt | 14 ++++--
.../download/HourPackageSyncTool.kt | 14 ++++--
.../download/DayPackageSyncToolTest.kt | 46 ++++++++++++++++++-
.../download/HourPackageSyncToolTest.kt | 46 ++++++++++++++++++-
.../main/home/HomeFragmentViewModelTest.kt | 11 ++---
6 files changed, 128 insertions(+), 25 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/BaseKeyPackageSyncTool.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/BaseKeyPackageSyncTool.kt
index 2cc60900d..a93bbcab0 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/BaseKeyPackageSyncTool.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/BaseKeyPackageSyncTool.kt
@@ -14,18 +14,28 @@ open class BaseKeyPackageSyncTool(
private val tag: String
) {
- internal suspend fun revokeCachedKeys(revokedKeyPackages: Collection<KeyDownloadConfig.RevokedKeyPackage>) {
+ /**
+ * Returns true if any of our cached keys were revoked
+ */
+ internal suspend fun revokeCachedKeys(
+ revokedKeyPackages: Collection<KeyDownloadConfig.RevokedKeyPackage>
+ ): Boolean {
if (revokedKeyPackages.isEmpty()) {
Timber.tag(tag).d("No revoked key packages to delete.")
- return
+ return false
}
val badEtags = revokedKeyPackages.map { it.etag }
- val toDelete = keyCache.getAllCachedKeys()
- .filter { badEtags.contains(it.info.etag) }
+ val toDelete = keyCache.getAllCachedKeys().filter { badEtags.contains(it.info.etag) }
- Timber.tag(tag).w("Deleting revoked cached keys: %s", toDelete.joinToString("\n"))
- keyCache.delete(toDelete.map { it.info })
+ return if (toDelete.isEmpty()) {
+ Timber.tag(tag).d("No local cached keys matched the revoked ones.")
+ false
+ } else {
+ Timber.tag(tag).w("Deleting revoked cached keys: %s", toDelete.joinToString("\n"))
+ keyCache.delete(toDelete.map { it.info })
+ true
+ }
}
internal suspend fun requireStorageSpace(data: List<LocationData>): DeviceStorage.CheckResult {
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncTool.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncTool.kt
index a0e56d389..9474d942d 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncTool.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncTool.kt
@@ -44,10 +44,10 @@ class DayPackageSyncTool @Inject constructor(
Timber.tag(TAG).v("syncMissingDays(targetLocations=%s)", targetLocations)
val downloadConfig: KeyDownloadConfig = configProvider.getAppConfig()
- revokeCachedKeys(downloadConfig.revokedDayPackages)
+ val keysWereRevoked = revokeCachedKeys(downloadConfig.revokedDayPackages)
val missingDays = targetLocations.mapNotNull {
- determineMissingDayPackages(it, forceIndexLookup)
+ determineMissingDayPackages(it, forceIndexLookup || keysWereRevoked)
}
if (missingDays.isEmpty()) {
Timber.tag(TAG).i("There were no missing day packages.")
@@ -80,10 +80,16 @@ class DayPackageSyncTool @Inject constructor(
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
- internal suspend fun determineMissingDayPackages(location: LocationCode, forceIndexLookup: Boolean): LocationDays? {
+ internal suspend fun determineMissingDayPackages(
+ location: LocationCode,
+ forceIndexLookup: Boolean
+ ): LocationDays? {
val cachedDays = getDownloadedCachedKeys(location, Type.LOCATION_DAY)
- if (!forceIndexLookup && !expectNewDayPackages(cachedDays)) return null
+ if (!forceIndexLookup && !expectNewDayPackages(cachedDays)) {
+ Timber.tag(TAG).d("We don't expect new day packages.")
+ return null
+ }
val availableDays = LocationDays(location, keyServer.getDayIndex(location))
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncTool.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncTool.kt
index c70080c32..ad63bbb13 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncTool.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncTool.kt
@@ -48,10 +48,10 @@ class HourPackageSyncTool @Inject constructor(
Timber.tag(TAG).v("syncMissingHours(targetLocations=%s)", targetLocations)
val downloadConfig: KeyDownloadConfig = configProvider.getAppConfig()
- revokeCachedKeys(downloadConfig.revokedHourPackages)
+ val keysWereRevoked = revokeCachedKeys(downloadConfig.revokedHourPackages)
val missingHours = targetLocations.mapNotNull {
- determineMissingHours(it, forceIndexLookup)
+ determineMissingHours(it, forceIndexLookup || keysWereRevoked)
}
if (missingHours.isEmpty()) {
Timber.tag(TAG).i("There were no missing hours.")
@@ -124,12 +124,18 @@ class HourPackageSyncTool @Inject constructor(
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
- internal suspend fun determineMissingHours(location: LocationCode, forceIndexLookup: Boolean): LocationHours? {
+ internal suspend fun determineMissingHours(
+ location: LocationCode,
+ forceIndexLookup: Boolean
+ ): LocationHours? {
val cachedHours = getDownloadedCachedKeys(location, Type.LOCATION_HOUR)
val now = timeStamper.nowUTC
- if (!forceIndexLookup && !expectNewHourPackages(cachedHours, now)) return null
+ if (!forceIndexLookup && !expectNewHourPackages(cachedHours, now)) {
+ Timber.tag(TAG).d("We don't expect new hour packages.")
+ return null
+ }
val today = now.toLocalDate()
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncToolTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncToolTest.kt
index 7db24597e..598e8408c 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncToolTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncToolTest.kt
@@ -6,6 +6,7 @@ import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo
import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo.Type
import io.kotest.matchers.shouldBe
import io.mockk.coEvery
+import io.mockk.coVerify
import io.mockk.coVerifySequence
import io.mockk.every
import io.mockk.mockk
@@ -82,7 +83,7 @@ class DayPackageSyncToolTest : CommonSyncToolTest() {
}
@Test
- fun `determine missing days forcesync ignores EXPECT NEW DAYS`() = runBlockingTest {
+ fun `determine missing days with forcesync ignores EXPECT NEW DAYS`() = runBlockingTest {
mockCachedDay("EUR".loc, "2020-01-01".day)
mockCachedDay("EUR".loc, "2020-01-02".day)
@@ -173,11 +174,52 @@ class DayPackageSyncToolTest : CommonSyncToolTest() {
keyCache.delete(listOf(invalidDay.info))
keyCache.getEntriesForType(Type.LOCATION_DAY)
- timeStamper.nowUTC
keyServer.getDayIndex("EUR".loc)
keyCache.createCacheEntry(Type.LOCATION_DAY, "EUR".loc, "2020-01-03".day, null)
downloadTool.downloadKeyFile(any(), downloadConfig)
}
}
+
+ @Test
+ fun `if keys were revoked skip the EXPECT packages check`() = runBlockingTest {
+ every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T12:12:12.000Z")
+ mockCachedDay("EUR".loc, "2020-01-01".day)
+ mockCachedDay("EUR".loc, "2020-01-02".day)
+ mockCachedDay("EUR".loc, "2020-01-03".day).apply {
+ every { downloadConfig.revokedDayPackages } returns listOf(
+ RevokedKeyPackage.Day(
+ day = info.day,
+ region = info.location,
+ etag = info.etag!!
+ )
+ )
+ }
+
+ createInstance().syncMissingDayPackages(listOf("EUR".loc), false)
+
+ coVerify(exactly = 1) { keyServer.getDayIndex("EUR".loc) }
+ }
+
+ @Test
+ fun `if force-sync is set we skip the EXPECT packages check`() = runBlockingTest {
+ every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T12:12:12.000Z")
+ mockCachedDay("EUR".loc, "2020-01-01".day)
+ mockCachedDay("EUR".loc, "2020-01-02".day)
+ mockCachedDay("EUR".loc, "2020-01-03".day)
+ createInstance().syncMissingDayPackages(listOf("EUR".loc), true)
+
+ coVerify(exactly = 1) { keyServer.getDayIndex("EUR".loc) }
+ }
+
+ @Test
+ fun `if neither force-sync is set and keys were revoked we check EXPECT NEW PKGS`() = runBlockingTest {
+ every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T12:12:12.000Z")
+ mockCachedDay("EUR".loc, "2020-01-01".day)
+ mockCachedDay("EUR".loc, "2020-01-02".day)
+ mockCachedDay("EUR".loc, "2020-01-03".day)
+ createInstance().syncMissingDayPackages(listOf("EUR".loc), false)
+
+ coVerify(exactly = 0) { keyServer.getDayIndex("EUR".loc) }
+ }
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncToolTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncToolTest.kt
index b2ca79f8b..062d1bfb4 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncToolTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncToolTest.kt
@@ -6,6 +6,7 @@ import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo
import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo.Type
import io.kotest.matchers.shouldBe
import io.mockk.coEvery
+import io.mockk.coVerify
import io.mockk.coVerifySequence
import io.mockk.every
import io.mockk.mockk
@@ -136,14 +137,14 @@ class HourPackageSyncToolTest : CommonSyncToolTest() {
}
@Test
- fun `determine missing hours forcesync ignores EXPECT NEW HOURS`() = runBlockingTest {
+ fun `determine missing hours with forcesync ignores EXPECT NEW HOURS`() = runBlockingTest {
mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
val instance = createInstance()
every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
- instance.determineMissingHours("EUR".loc, true) shouldBe LocationHours(
+ instance.determineMissingHours("EUR".loc, forceIndexLookup = true) shouldBe LocationHours(
location = "EUR".loc,
hourData = mapOf("2020-01-04".day to listOf("02:00".hour))
)
@@ -202,4 +203,45 @@ class HourPackageSyncToolTest : CommonSyncToolTest() {
now = Instant.parse("2020-01-01T03:00:03.000Z")
instance.expectNewHourPackages(listOf(cachedKey1, cachedKey2), now) shouldBe true
}
+
+ @Test
+ fun `if keys were revoked skip the EXPECT packages check`() = runBlockingTest {
+ every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
+ mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
+ mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
+ mockCachedHour("EUR".loc, "2020-01-04".day, "02:00".hour).apply {
+ every { downloadConfig.revokedHourPackages } returns listOf(
+ RevokedKeyPackage.Hour(
+ region = info.location,
+ etag = info.etag!!,
+ day = info.day,
+ hour = info.hour!!
+ )
+ )
+ }
+
+ createInstance().syncMissingHourPackages(listOf("EUR".loc), false)
+
+ coVerify(exactly = 1) { keyServer.getHourIndex("EUR".loc, "2020-01-04".day) }
+ }
+
+ @Test
+ fun `if force-sync is set we skip the EXPECT packages check`() = runBlockingTest {
+ every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
+ mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
+ mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
+ createInstance().syncMissingHourPackages(listOf("EUR".loc), true)
+
+ coVerify(exactly = 1) { keyServer.getHourIndex("EUR".loc, "2020-01-04".day) }
+ }
+
+ @Test
+ fun `if neither force-sync is set and keys were revoked we check EXPECT NEW PKGS`() = runBlockingTest {
+ every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
+ mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
+ mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
+ createInstance().syncMissingHourPackages(listOf("EUR".loc), false)
+
+ coVerify(exactly = 0) { keyServer.getHourIndex("EUR".loc, "2020-01-04".day) }
+ }
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/main/home/HomeFragmentViewModelTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/main/home/HomeFragmentViewModelTest.kt
index f2e40787e..44a4980d7 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/main/home/HomeFragmentViewModelTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/main/home/HomeFragmentViewModelTest.kt
@@ -9,16 +9,13 @@ import de.rki.coronawarnapp.ui.main.home.HomeFragmentViewModel
import de.rki.coronawarnapp.ui.main.home.SubmissionCardState
import de.rki.coronawarnapp.ui.main.home.SubmissionCardsStateProvider
import de.rki.coronawarnapp.ui.main.home.TracingHeaderState
-import de.rki.coronawarnapp.ui.submission.ApiRequestState
import de.rki.coronawarnapp.ui.submission.ApiRequestState.SUCCESS
import de.rki.coronawarnapp.ui.tracing.card.TracingCardState
import de.rki.coronawarnapp.ui.tracing.card.TracingCardStateProvider
import de.rki.coronawarnapp.ui.viewmodel.SettingsViewModel
-import de.rki.coronawarnapp.util.DeviceUIState
import de.rki.coronawarnapp.util.DeviceUIState.PAIRED_POSITIVE
import de.rki.coronawarnapp.util.DeviceUIState.PAIRED_POSITIVE_TELETAN
import de.rki.coronawarnapp.util.security.EncryptionErrorResetTool
-import io.kotest.matchers.neverNullMatcher
import io.kotest.matchers.shouldBe
import io.mockk.MockKAnnotations
import io.mockk.clearAllMocks
@@ -133,12 +130,12 @@ class HomeFragmentViewModelTest : BaseTest() {
fun `positive test result notification is triggered on positive QR code result`() {
val state = SubmissionCardState(PAIRED_POSITIVE, true, SUCCESS)
every { submissionCardsStateProvider.state } returns flowOf(state)
- every {testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
+ every { testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
runBlocking {
createInstance().apply {
observeTestResultToSchedulePositiveTestResultReminder()
- verify { testResultNotificationService.schedulePositiveTestResultReminder()}
+ verify { testResultNotificationService.schedulePositiveTestResultReminder() }
}
}
}
@@ -147,12 +144,12 @@ class HomeFragmentViewModelTest : BaseTest() {
fun `positive test result notification is triggered on positive TeleTan code result`() {
val state = SubmissionCardState(PAIRED_POSITIVE_TELETAN, true, SUCCESS)
every { submissionCardsStateProvider.state } returns flowOf(state)
- every {testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
+ every { testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
runBlocking {
createInstance().apply {
observeTestResultToSchedulePositiveTestResultReminder()
- verify { testResultNotificationService.schedulePositiveTestResultReminder()}
+ verify { testResultNotificationService.schedulePositiveTestResultReminder() }
}
}
}
--
GitLab