From 654bdb6fb0b5c04e96377ac5bef391324856b134 Mon Sep 17 00:00:00 2001 From: Matthias Urhahn Date: Mon, 12 Apr 2021 16:46:47 +0200 Subject: [PATCH] Prevent matching our own CheckIns (EXPOSUREAPP-6305) (#2797) * Prevent matching our own CheckIns, by marking submitted CheckIns in the database and skipping them during matching. * Add missing value. Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> Co-authored-by: Juraj Kusnier --- .../1.json | 12 +++- .../storage/CheckInDatabaseData.kt | 6 +- .../eventregistration/checkins/CheckIn.kt | 6 +- .../checkins/CheckInRepository.kt | 8 +++ .../storage/dao/CheckInDao.kt | 3 + .../entity/TraceLocationCheckInEntity.kt | 15 ++++- .../risk/calculation/CheckInWarningMatcher.kt | 5 ++ .../submission/task/SubmissionTask.kt | 10 ++++ .../checkins/CheckInRepositoryTest.kt | 12 ++-- .../calculation/CheckInWarningMatcherTest.kt | 55 +++++++++++++++++++ .../risk/calculation/OverlapTest.kt | 24 +++++++- .../submission/task/SubmissionTaskTest.kt | 24 +++++++- 12 files changed, 163 insertions(+), 17 deletions(-) diff --git a/Corona-Warn-App/schemas/de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase/1.json b/Corona-Warn-App/schemas/de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase/1.json index befd229435a..856916f58df 100644 --- a/Corona-Warn-App/schemas/de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase/1.json +++ b/Corona-Warn-App/schemas/de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase/1.json @@ -2,11 +2,11 @@ "formatVersion": 1, "database": { "version": 1, - "identityHash": "5117ed4caaa7ecd70051902d844cc665", + "identityHash": "e23913768d43dc0cb1374df83b2fa78a", "entities": [ { "tableName": "checkin", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `traceLocationIdBase64` TEXT NOT NULL, `version` INTEGER NOT NULL, `type` INTEGER NOT NULL, `description` TEXT NOT NULL, `address` TEXT NOT NULL, `traceLocationStart` TEXT, `traceLocationEnd` TEXT, `defaultCheckInLengthInMinutes` INTEGER, `cryptographicSeedBase64` TEXT NOT NULL, `cnPublicKey` TEXT NOT NULL, `checkInStart` TEXT NOT NULL, `checkInEnd` TEXT NOT NULL, `completed` INTEGER NOT NULL, `createJournalEntry` INTEGER NOT NULL)", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `traceLocationIdBase64` TEXT NOT NULL, `version` INTEGER NOT NULL, `type` INTEGER NOT NULL, `description` TEXT NOT NULL, `address` TEXT NOT NULL, `traceLocationStart` TEXT, `traceLocationEnd` TEXT, `defaultCheckInLengthInMinutes` INTEGER, `cryptographicSeedBase64` TEXT NOT NULL, `cnPublicKey` TEXT NOT NULL, `checkInStart` TEXT NOT NULL, `checkInEnd` TEXT NOT NULL, `completed` INTEGER NOT NULL, `createJournalEntry` INTEGER NOT NULL, `submitted` INTEGER NOT NULL)", "fields": [ { "fieldPath": "id", @@ -97,6 +97,12 @@ "columnName": "createJournalEntry", "affinity": "INTEGER", "notNull": true + }, + { + "fieldPath": "isSubmitted", + "columnName": "submitted", + "affinity": "INTEGER", + "notNull": true } ], "primaryKey": { @@ -186,7 +192,7 @@ "views": [], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '5117ed4caaa7ecd70051902d844cc665')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'e23913768d43dc0cb1374df83b2fa78a')" ] } } \ No newline at end of file diff --git a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/eventregistration/storage/CheckInDatabaseData.kt b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/eventregistration/storage/CheckInDatabaseData.kt index 370276daf4f..1ab9a7725c5 100644 --- a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/eventregistration/storage/CheckInDatabaseData.kt +++ b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/eventregistration/storage/CheckInDatabaseData.kt @@ -21,7 +21,8 @@ object CheckInDatabaseData { checkInStart = Instant.parse("2021-01-01T12:30:00.000Z"), checkInEnd = Instant.parse("2021-01-01T14:00:00.000Z"), completed = false, - createJournalEntry = true + createJournalEntry = true, + isSubmitted = false, ) val testCheckInWithoutCheckOutTime = TraceLocationCheckInEntity( @@ -38,6 +39,7 @@ object CheckInDatabaseData { checkInStart = Instant.parse("2021-01-01T12:30:00.000Z"), checkInEnd = Instant.parse("2021-01-01T14:00:00.000Z"), completed = false, - createJournalEntry = true + createJournalEntry = true, + isSubmitted = false, ) } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckIn.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckIn.kt index 1f755923ad6..8fea0b27af2 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckIn.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckIn.kt @@ -22,7 +22,8 @@ data class CheckIn( val checkInStart: Instant, val checkInEnd: Instant, val completed: Boolean, - val createJournalEntry: Boolean + val createJournalEntry: Boolean, + val isSubmitted: Boolean = false, ) { /** * Returns SHA-256 hash of [traceLocationId] which itself may also be SHA-256 hash. @@ -48,5 +49,6 @@ fun CheckIn.toEntity() = TraceLocationCheckInEntity( checkInStart = checkInStart, checkInEnd = checkInEnd, completed = completed, - createJournalEntry = createJournalEntry + createJournalEntry = createJournalEntry, + isSubmitted = isSubmitted, ) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepository.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepository.kt index dbac99970a4..ea9d8b74b00 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepository.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepository.kt @@ -2,6 +2,7 @@ package de.rki.coronawarnapp.eventregistration.checkins import de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase import de.rki.coronawarnapp.eventregistration.storage.dao.CheckInDao +import de.rki.coronawarnapp.eventregistration.storage.entity.TraceLocationCheckInEntity import de.rki.coronawarnapp.eventregistration.storage.entity.toCheckIn import de.rki.coronawarnapp.util.TimeStamper import kotlinx.coroutines.NonCancellable @@ -64,6 +65,13 @@ class CheckInRepository @Inject constructor( checkInDao.updateEntityById(checkInId, update) } + suspend fun markCheckInAsSubmitted(checkInId: Long) { + Timber.d("markCheckInAsSubmitted(checkInId=$checkInId)") + checkInDao.updateEntity( + TraceLocationCheckInEntity.SubmissionUpdate(checkInId = checkInId, isSubmitted = true) + ) + } + suspend fun deleteCheckIns(checkIns: Collection) = withContext(NonCancellable) { Timber.d("deleteCheckIns(checkIns=%s)", checkIns) checkInDao.deleteByIds(checkIns.map { it.id }) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/dao/CheckInDao.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/dao/CheckInDao.kt index b45d75d1b09..56ed45b2abe 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/dao/CheckInDao.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/dao/CheckInDao.kt @@ -37,6 +37,9 @@ interface CheckInDao { update(updated) } + @Update(entity = TraceLocationCheckInEntity::class) + suspend fun updateEntity(update: TraceLocationCheckInEntity.SubmissionUpdate) + @Query("DELETE FROM checkin") suspend fun deleteAll() diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/entity/TraceLocationCheckInEntity.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/entity/TraceLocationCheckInEntity.kt index 2616685c8a0..8ddcadd2a8b 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/entity/TraceLocationCheckInEntity.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/entity/TraceLocationCheckInEntity.kt @@ -23,8 +23,16 @@ data class TraceLocationCheckInEntity( @ColumnInfo(name = "checkInStart") val checkInStart: Instant, @ColumnInfo(name = "checkInEnd") val checkInEnd: Instant, @ColumnInfo(name = "completed") val completed: Boolean, - @ColumnInfo(name = "createJournalEntry") val createJournalEntry: Boolean -) + @ColumnInfo(name = "createJournalEntry") val createJournalEntry: Boolean, + @ColumnInfo(name = "submitted") val isSubmitted: Boolean, +) { + + @Entity + data class SubmissionUpdate( + @PrimaryKey @ColumnInfo(name = "id") val checkInId: Long, + @ColumnInfo(name = "submitted") val isSubmitted: Boolean, + ) +} fun TraceLocationCheckInEntity.toCheckIn() = CheckIn( id = id, @@ -41,5 +49,6 @@ fun TraceLocationCheckInEntity.toCheckIn() = CheckIn( checkInStart = checkInStart, checkInEnd = checkInEnd, completed = completed, - createJournalEntry = createJournalEntry + createJournalEntry = createJournalEntry, + isSubmitted = isSubmitted ) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcher.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcher.kt index 8dce26bef83..530ebc13807 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcher.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcher.kt @@ -135,6 +135,11 @@ internal fun CheckIn.calculateOverlap( return null } + if (isSubmitted) { + Timber.tag(TAG).d("Overlap with our own CheckIn (%s and %s)", this, warning) + return null + } + return CheckInWarningOverlap( checkInId = id, transmissionRiskLevel = warning.transmissionRiskLevel, diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/submission/task/SubmissionTask.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/submission/task/SubmissionTask.kt index 4613a92bc27..5f801d507b6 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/submission/task/SubmissionTask.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/submission/task/SubmissionTask.kt @@ -2,6 +2,7 @@ package de.rki.coronawarnapp.submission.task import com.google.android.gms.nearby.exposurenotification.TemporaryExposureKey import de.rki.coronawarnapp.appconfig.AppConfigProvider +import de.rki.coronawarnapp.bugreporting.reportProblem import de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission.AnalyticsKeySubmissionCollector import de.rki.coronawarnapp.eventregistration.checkins.CheckInRepository import de.rki.coronawarnapp.eventregistration.checkins.CheckInsTransformer @@ -167,6 +168,15 @@ class SubmissionTask @Inject constructor( tekHistoryStorage.clear() submissionSettings.symptoms.update { null } + Timber.tag(TAG).d("Marking %d submitted CheckIns.", checkIns.size) + checkIns.forEach { checkIn -> + try { + checkInsRepository.markCheckInAsSubmitted(checkIn.id) + } catch (e: Exception) { + e.reportProblem(TAG, "CheckIn $checkIn could not be marked as submitted") + } + } + autoSubmission.updateMode(AutoSubmission.Mode.DISABLED) setSubmissionFinished() diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepositoryTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepositoryTest.kt index 162c73db7a2..23ea1802a9c 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepositoryTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepositoryTest.kt @@ -92,7 +92,8 @@ class CheckInRepositoryTest : BaseTest() { checkInStart = time, checkInEnd = end, completed = false, - createJournalEntry = false + createJournalEntry = false, + isSubmitted = true ) ) coVerify { @@ -112,7 +113,8 @@ class CheckInRepositoryTest : BaseTest() { checkInStart = time, checkInEnd = end, completed = false, - createJournalEntry = false + createJournalEntry = false, + isSubmitted = true, ) ) } @@ -156,7 +158,8 @@ class CheckInRepositoryTest : BaseTest() { checkInStart = start, checkInEnd = end, completed = false, - createJournalEntry = false + createJournalEntry = false, + isSubmitted = true, ) ) runBlockingTest { @@ -176,7 +179,8 @@ class CheckInRepositoryTest : BaseTest() { checkInStart = start, checkInEnd = end, completed = false, - createJournalEntry = false + createJournalEntry = false, + isSubmitted = true, ) ) } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcherTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcherTest.kt index b8beb8b1106..52e215794f5 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcherTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/CheckInWarningMatcherTest.kt @@ -252,4 +252,59 @@ class CheckInWarningMatcherTest : BaseTest() { } } } + + @Test + fun `we do not match our own CheckIns`() { + val checkIn1 = createCheckIn( + id = 2L, + traceLocationId = "fe84394e73838590cc7707aba0350c130f6d0fb6f0f2535f9735f481dee61871", + startDateStr = "2021-03-04T10:15+01:00", + endDateStr = "2021-03-04T10:17+01:00" + ) + val checkIn2 = createCheckIn( + id = 3L, + traceLocationId = "69eb427e1a48133970486244487e31b3f1c5bde47415db9b52cc5a2ece1e0060", + startDateStr = "2021-03-04T09:15+01:00", + endDateStr = "2021-03-04T10:12+01:00", + isSubmitted = true + ) + + val warning1 = createWarning( + traceLocationId = "fe84394e73838590cc7707aba0350c130f6d0fb6f0f2535f9735f481dee61871", + startIntervalDateStr = "2021-03-04T10:00+01:00", + period = 6, + transmissionRiskLevel = 8 + ) + + val warning2 = createWarning( + traceLocationId = "69eb427e1a48133970486244487e31b3f1c5bde47415db9b52cc5a2ece1e0060", + startIntervalDateStr = "2021-03-04T10:00+01:00", + period = 6, + transmissionRiskLevel = 8 + ) + + val warningPackage = object : TraceWarningPackage { + override suspend fun extractWarnings(): List { + return listOf(warning1, warning2) + } + + override val packageId: WarningPackageId + get() = "id" + } + + runBlockingTest { + val result = createInstance().process( + checkIns = listOf(checkIn1, checkIn2), + warningPackages = listOf(warningPackage) + ) + result.apply { + successful shouldBe true + processedPackages.single().warningPackage shouldBe warningPackage + processedPackages.single().apply { + overlaps.size shouldBe 1 + overlaps.any { it.checkInId == 2L } shouldBe true + } + } + } + } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/OverlapTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/OverlapTest.kt index 67cdb5a057b..bff78e2cc5b 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/OverlapTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/calculation/OverlapTest.kt @@ -262,13 +262,32 @@ class OverlapTest : BaseTest() { traceWarningPackageId = id )!!.roundedMinutes shouldBe 5 } + + @Test + fun `returns null if it matches our own ssubmitted CheckIn`() { + createCheckIn( + traceLocationId = locationId, + startDateStr = "2021-03-04T10:15+01:00", + endDateStr = "2021-03-04T10:17+01:00", + isSubmitted = true, + ).calculateOverlap( + createWarning( + traceLocationId = locationId, + startIntervalDateStr = "2021-03-04T10:00+01:00", + period = 6, + transmissionRiskLevel = 8 + ), + traceWarningPackageId = id + ) shouldBe null + } } fun createCheckIn( id: Long = 1L, traceLocationId: String, startDateStr: String, - endDateStr: String + endDateStr: String, + isSubmitted: Boolean = false, ) = CheckIn( id = id, traceLocationId = traceLocationId.decodeHex(), @@ -284,7 +303,8 @@ fun createCheckIn( checkInStart = Instant.parse(startDateStr), checkInEnd = Instant.parse(endDateStr), completed = false, - createJournalEntry = false + createJournalEntry = false, + isSubmitted = isSubmitted ) fun createWarning( diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/submission/task/SubmissionTaskTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/submission/task/SubmissionTaskTest.kt index b18c793f78e..778d26c72e6 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/submission/task/SubmissionTaskTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/submission/task/SubmissionTaskTest.kt @@ -4,6 +4,7 @@ import com.google.android.gms.nearby.exposurenotification.TemporaryExposureKey import de.rki.coronawarnapp.appconfig.AppConfigProvider import de.rki.coronawarnapp.appconfig.ConfigData import de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission.AnalyticsKeySubmissionCollector +import de.rki.coronawarnapp.eventregistration.checkins.CheckIn import de.rki.coronawarnapp.eventregistration.checkins.CheckInRepository import de.rki.coronawarnapp.eventregistration.checkins.CheckInsTransformer import de.rki.coronawarnapp.exception.NoRegistrationTokenSetException @@ -73,6 +74,25 @@ class SubmissionTaskTest : BaseTest() { private val settingLastUserActivityUTC: FlowPreference = mockFlowPreference(Instant.EPOCH.plus(1)) + private val testCheckIn1 = CheckIn( + id = 1L, + traceLocationId = mockk(), + version = 1, + type = 2, + description = "brothers birthday", + address = "Malibu", + traceLocationStart = Instant.EPOCH, + traceLocationEnd = null, + defaultCheckInLengthInMinutes = null, + cryptographicSeed = mockk(), + cnPublicKey = "cnPublicKey", + checkInStart = Instant.EPOCH, + checkInEnd = Instant.EPOCH.plus(9000), + completed = false, + createJournalEntry = false, + isSubmitted = true + ) + @BeforeEach fun setup() { MockKAnnotations.init(this) @@ -113,7 +133,7 @@ class SubmissionTaskTest : BaseTest() { every { timeStamper.nowUTC } returns Instant.EPOCH.plus(Duration.standardHours(1)) - every { checkInRepository.checkInsWithinRetention } returns flowOf(emptyList()) + every { checkInRepository.checkInsWithinRetention } returns flowOf(listOf(testCheckIn1)) coEvery { checkInsTransformer.transform(any(), any()) } returns emptyList() } @@ -179,6 +199,8 @@ class SubmissionTaskTest : BaseTest() { submissionSettings.symptoms settingSymptomsPreference.update(match { it.invoke(mockk()) == null }) + checkInRepository.markCheckInAsSubmitted(testCheckIn1.id) + autoSubmission.updateMode(AutoSubmission.Mode.DISABLED) backgroundWorkScheduler.stopWorkScheduler()