From 1b9e37c9a54fa2fcc7a0764fd85c3a0e5d659d88 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Wed, 25 Nov 2020 15:02:32 +0100
Subject: [PATCH] Prevent accidental use of protobuf default value for app
features (DEV) (#1716)
* Make use of `hasAppFeatures` to prevent accidental use of default values.
* Swap order in if statement to remove unnecessary negation
* Reduce visibility
Co-authored-by: ralfgehrer <mail@ralfgehrer.com>
Co-authored-by: Kolya Opahle <k.opahle@sap.com>
Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com>
---
.../rki/coronawarnapp/appconfig/CWAConfig.kt | 2 +-
.../appconfig/mapping/CWAConfigMapper.kt | 23 ++++++++++++------
.../appconfig/mapping/CWAConfigMapperTest.kt | 24 +++++++++++++++++++
3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/CWAConfig.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/CWAConfig.kt
index 89532fb14..e0a59c5c8 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/CWAConfig.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/CWAConfig.kt
@@ -11,7 +11,7 @@ interface CWAConfig {
val supportedCountries: List<String>
- val appFeatures: AppFeaturesOuterClass.AppFeatures
+ val appFeatures: List<AppFeaturesOuterClass.AppFeature>
interface Mapper : ConfigMapper<CWAConfig>
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapper.kt
index 8c2e9502f..47b827428 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapper.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapper.kt
@@ -1,26 +1,24 @@
package de.rki.coronawarnapp.appconfig.mapping
-import androidx.annotation.VisibleForTesting
import dagger.Reusable
import de.rki.coronawarnapp.appconfig.CWAConfig
-import de.rki.coronawarnapp.server.protocols.internal.v2.AppConfigAndroid
+import de.rki.coronawarnapp.server.protocols.internal.v2.AppConfigAndroid.ApplicationConfigurationAndroid
import de.rki.coronawarnapp.server.protocols.internal.v2.AppFeaturesOuterClass
import timber.log.Timber
import javax.inject.Inject
@Reusable
class CWAConfigMapper @Inject constructor() : CWAConfig.Mapper {
- override fun map(rawConfig: AppConfigAndroid.ApplicationConfigurationAndroid): CWAConfig {
+ override fun map(rawConfig: ApplicationConfigurationAndroid): CWAConfig {
return CWAConfigContainer(
latestVersionCode = rawConfig.latestVersionCode,
minVersionCode = rawConfig.minVersionCode,
supportedCountries = rawConfig.getMappedSupportedCountries(),
- appFeatures = rawConfig.appFeatures
+ appFeatures = rawConfig.mapAppFeatures()
)
}
- @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
- internal fun AppConfigAndroid.ApplicationConfigurationAndroid.getMappedSupportedCountries(): List<String> =
+ private fun ApplicationConfigurationAndroid.getMappedSupportedCountries(): List<String> =
when {
supportedCountriesList == null -> emptyList()
supportedCountriesList.size == 1 && !VALID_CC.matches(supportedCountriesList.single()) -> {
@@ -30,11 +28,22 @@ class CWAConfigMapper @Inject constructor() : CWAConfig.Mapper {
else -> supportedCountriesList
}
+ private fun ApplicationConfigurationAndroid.mapAppFeatures(): List<AppFeaturesOuterClass.AppFeature> =
+ if (hasAppFeatures()) {
+ val parsedFeatures = mutableListOf<AppFeaturesOuterClass.AppFeature>()
+ for (index in 0 until appFeatures.appFeaturesCount) {
+ parsedFeatures.add(appFeatures.getAppFeatures(index))
+ }
+ parsedFeatures
+ } else {
+ emptyList()
+ }
+
data class CWAConfigContainer(
override val latestVersionCode: Long,
override val minVersionCode: Long,
override val supportedCountries: List<String>,
- override val appFeatures: AppFeaturesOuterClass.AppFeatures
+ override val appFeatures: List<AppFeaturesOuterClass.AppFeature>
) : CWAConfig
companion object {
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapperTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapperTest.kt
index edebc20fd..33977018e 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapperTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/CWAConfigMapperTest.kt
@@ -1,6 +1,7 @@
package de.rki.coronawarnapp.appconfig.mapping
import de.rki.coronawarnapp.server.protocols.internal.v2.AppConfigAndroid
+import de.rki.coronawarnapp.server.protocols.internal.v2.AppFeaturesOuterClass
import io.kotest.matchers.shouldBe
import org.junit.jupiter.api.Test
import testhelpers.BaseTest
@@ -45,4 +46,27 @@ class CWAConfigMapperTest : BaseTest() {
this.supportedCountries shouldBe emptyList()
}
}
+
+ @Test
+ fun `app features are mapped`() {
+ val rawConfig = AppConfigAndroid.ApplicationConfigurationAndroid.newBuilder()
+ .setAppFeatures(
+ AppFeaturesOuterClass.AppFeatures.newBuilder().apply {
+ addAppFeatures(AppFeaturesOuterClass.AppFeature.newBuilder().apply { }.build())
+ }
+ )
+ .build()
+ createInstance().map(rawConfig).apply {
+ appFeatures.size shouldBe 1
+ }
+ }
+
+ @Test
+ fun `app features being empty are handled`() {
+ val rawConfig = AppConfigAndroid.ApplicationConfigurationAndroid.newBuilder()
+ .build()
+ createInstance().map(rawConfig).apply {
+ appFeatures shouldBe emptyList()
+ }
+ }
}
--
GitLab