From 10694530302f8da3f05d276e3520b5684b0fd89a Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Wed, 10 Feb 2021 17:20:22 +0100 Subject: [PATCH] Adjust device time test setting. (#2316) Instead of faking the "disable-device-time-check" which won't work for SafetyNet, we now fake the time offset to be within the acceptable delta. Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../appconfig/ui/AppConfigTestFragment.kt | 8 +-- .../ui/AppConfigTestFragmentViewModel.kt | 6 +- .../ui/DataDonationTestFragment.kt | 5 +- .../res/layout/fragment_test_appconfig.xml | 10 ++-- .../appconfig/mapping/CWAConfigMapper.kt | 10 +--- .../sources/remote/AppConfigServer.kt | 12 +++- .../sources/remote/RemoteAppConfigSource.kt | 2 +- .../rki/coronawarnapp/storage/TestSettings.kt | 9 ++- .../appconfig/mapping/CWAConfigMapperTest.kt | 23 +------- .../sources/remote/AppConfigServerTest.kt | 56 ++++++++++++++++++- 10 files changed, 89 insertions(+), 52 deletions(-) diff --git a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragment.kt b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragment.kt index 6ca077608..8a0f03194 100644 --- a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragment.kt +++ b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragment.kt @@ -48,11 +48,11 @@ class AppConfigTestFragment : Fragment(R.layout.fragment_test_appconfig), AutoIn binding.downloadAction.setOnClickListener { vm.download() } binding.deleteAction.setOnClickListener { vm.clearConfig() } - vm.deviceTimeCheckDisabled.observe2(this) { - binding.deviceTimeCheckDisabledToggle.isChecked = it + vm.isDeviceTimeFaked.observe2(this) { + binding.fakeCorrectDevicetimeToggle.isChecked = it } - binding.deviceTimeCheckDisabledToggle.setOnClickListener { - vm.toggleDeviceTimeCheckDisabled() + binding.fakeCorrectDevicetimeToggle.setOnClickListener { + vm.toggleFakeCorrectDeviceTime() } } diff --git a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragmentViewModel.kt b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragmentViewModel.kt index 1f98cae3f..8b4a2217b 100644 --- a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragmentViewModel.kt +++ b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/appconfig/ui/AppConfigTestFragmentViewModel.kt @@ -20,7 +20,7 @@ class AppConfigTestFragmentViewModel @AssistedInject constructor( val currentConfig = appConfigProvider.currentConfig.asLiveData() val errorEvent = SingleLiveEvent<Exception>() - val deviceTimeCheckDisabled = testSettings.isDeviceTimeCheckDisabled.flow + val isDeviceTimeFaked = testSettings.fakeCorrectDeviceTime.flow .asLiveData(context = dispatcherProvider.Default) fun download() { @@ -40,8 +40,8 @@ class AppConfigTestFragmentViewModel @AssistedInject constructor( } } - fun toggleDeviceTimeCheckDisabled() { - testSettings.isDeviceTimeCheckDisabled.update { !it } + fun toggleFakeCorrectDeviceTime() { + testSettings.fakeCorrectDeviceTime.update { !it } } @AssistedFactory diff --git a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragment.kt b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragment.kt index eac75a127..2a6691044 100644 --- a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragment.kt +++ b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragment.kt @@ -55,8 +55,11 @@ class DataDonationTestFragment : Fragment(R.layout.fragment_test_datadonation), } binding.safetynetRequirementsBody.apply { text = items.first.toString() + append("\n\n") if (items.second != null) { - append("\n\n" + items.second.toString()) + append(items.second.toString()) + } else { + append("Requirements passed!") } } } diff --git a/Corona-Warn-App/src/deviceForTesters/res/layout/fragment_test_appconfig.xml b/Corona-Warn-App/src/deviceForTesters/res/layout/fragment_test_appconfig.xml index 6727ba483..ad078e5bc 100644 --- a/Corona-Warn-App/src/deviceForTesters/res/layout/fragment_test_appconfig.xml +++ b/Corona-Warn-App/src/deviceForTesters/res/layout/fragment_test_appconfig.xml @@ -4,6 +4,7 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="match_parent" + xmlns:app="http://schemas.android.com/apk/res-auto" tools:ignore="HardcodedText"> <LinearLayout @@ -80,15 +81,14 @@ android:layout_margin="@dimen/spacing_tiny" android:orientation="vertical"> - <Switch - android:id="@+id/device_time_check_disabled_toggle" - style="@style/body1" + <com.google.android.material.switchmaterial.SwitchMaterial + android:id="@+id/fake_correct_devicetime_toggle" android:layout_width="match_parent" + app:thumbTint="@color/colorAccent" android:layout_height="0dp" android:layout_marginTop="@dimen/spacing_tiny" android:layout_weight="1" - android:text="Disable device time check" - android:theme="@style/switchBase" /> + android:text="Fake correct device time" /> </LinearLayout> 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 830989127..cd869dea6 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 @@ -3,15 +3,11 @@ package de.rki.coronawarnapp.appconfig.mapping import dagger.Reusable import de.rki.coronawarnapp.appconfig.CWAConfig import de.rki.coronawarnapp.server.protocols.internal.v2.AppConfigAndroid.ApplicationConfigurationAndroid -import de.rki.coronawarnapp.storage.TestSettings -import de.rki.coronawarnapp.util.CWADebug import timber.log.Timber import javax.inject.Inject @Reusable -class CWAConfigMapper @Inject constructor( - private val testSettings: TestSettings -) : CWAConfig.Mapper { +class CWAConfigMapper @Inject constructor() : CWAConfig.Mapper { override fun map(rawConfig: ApplicationConfigurationAndroid): CWAConfig { return CWAConfigContainer( @@ -33,10 +29,6 @@ class CWAConfigMapper @Inject constructor( } private fun ApplicationConfigurationAndroid.isDeviceTimeCheckDisabled(): Boolean { - if (CWADebug.isDeviceForTestersBuild && testSettings.isDeviceTimeCheckDisabled.value) { - Timber.w("Device time check is disabled via test settings!") - return true - } if (!hasAppFeatures()) return false return try { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt index b18ba06e8..334ef2102 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt @@ -6,6 +6,8 @@ import de.rki.coronawarnapp.appconfig.download.AppConfigApiV2 import de.rki.coronawarnapp.appconfig.internal.ApplicationConfigurationCorruptException import de.rki.coronawarnapp.appconfig.internal.ApplicationConfigurationInvalidException import de.rki.coronawarnapp.appconfig.internal.InternalConfigData +import de.rki.coronawarnapp.storage.TestSettings +import de.rki.coronawarnapp.util.CWADebug import de.rki.coronawarnapp.util.TimeStamper import de.rki.coronawarnapp.util.ZipHelper.readIntoMap import de.rki.coronawarnapp.util.ZipHelper.unzip @@ -25,7 +27,8 @@ import javax.inject.Inject class AppConfigServer @Inject constructor( private val api: Lazy<AppConfigApiV2>, private val verificationKeys: VerificationKeys, - private val timeStamper: TimeStamper + private val timeStamper: TimeStamper, + private val testSettings: TestSettings ) { internal suspend fun downloadAppConfig(): InternalConfigData { @@ -62,7 +65,12 @@ class AppConfigServer @Inject constructor( ?: throw ApplicationConfigurationInvalidException(message = "Server has no ETAG.") val serverTime = response.getServerDate() ?: localTime - val offset = Duration(serverTime, localTime) + val offset = if (CWADebug.isDeviceForTestersBuild && testSettings.fakeCorrectDeviceTime.value) { + Timber.tag(TAG).w("Test setting 'fakeCorrectDeviceTime' is active; time offset is now +90min.") + Duration.standardMinutes(90) + } else { + Duration(serverTime, localTime) + } Timber.tag(TAG).v("Time offset was %dms", offset.millis) val cacheControl = CacheControl.parse(headers) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt index 6c4007a7b..0af12fdf7 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt @@ -42,7 +42,7 @@ class RemoteAppConfigSource @Inject constructor( ) } } catch (e: Exception) { - Timber.tag(TAG).e(e, "Failed to parse AppConfig from server, trying fallback.") + Timber.tag(TAG).e(e, "Failed to parse AppConfig from server.") null } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/TestSettings.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/TestSettings.kt index ccf131c69..e9bb38c84 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/TestSettings.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/TestSettings.kt @@ -19,8 +19,8 @@ class TestSettings @Inject constructor( context.getSharedPreferences("test_settings", Context.MODE_PRIVATE) } - val isDeviceTimeCheckDisabled = prefs.createFlowPreference( - key = "config.devicetimecheck.disabled", + val fakeCorrectDeviceTime = prefs.createFlowPreference( + key = "config.devicetimecheck.fake.correct", defaultValue = false ) @@ -29,10 +29,9 @@ class TestSettings @Inject constructor( defaultValue = false ) - val fakeExposureWindows = FlowPreference( - preferences = prefs, + val fakeExposureWindows: FlowPreference<FakeExposureWindowTypes> = prefs.createFlowPreference( key = "riskleve.exposurewindows.fake", - reader = FlowPreference.gsonReader<FakeExposureWindowTypes>(gson, FakeExposureWindowTypes.DISABLED), + reader = FlowPreference.gsonReader(gson, FakeExposureWindowTypes.DISABLED), writer = FlowPreference.gsonWriter(gson) ) 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 3b4dbd04f..e00fac30f 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 @@ -3,35 +3,27 @@ package de.rki.coronawarnapp.appconfig.mapping import de.rki.coronawarnapp.server.protocols.internal.v2.AppConfigAndroid.ApplicationConfigurationAndroid import de.rki.coronawarnapp.server.protocols.internal.v2.AppFeaturesOuterClass.AppFeature import de.rki.coronawarnapp.server.protocols.internal.v2.AppFeaturesOuterClass.AppFeatures -import de.rki.coronawarnapp.storage.TestSettings import de.rki.coronawarnapp.util.CWADebug import io.kotest.matchers.shouldBe import io.mockk.MockKAnnotations import io.mockk.clearAllMocks import io.mockk.every -import io.mockk.impl.annotations.MockK import io.mockk.mockkObject import io.mockk.spyk import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import testhelpers.BaseTest -import testhelpers.preferences.mockFlowPreference class CWAConfigMapperTest : BaseTest() { - @MockK lateinit var testSettings: TestSettings - - private fun createInstance() = CWAConfigMapper( - testSettings = testSettings - ) + private fun createInstance() = CWAConfigMapper() @BeforeEach fun setup() { MockKAnnotations.init(this) mockkObject(CWADebug) - every { testSettings.isDeviceTimeCheckDisabled } returns mockFlowPreference(false) } @AfterEach @@ -156,7 +148,7 @@ class CWAConfigMapperTest : BaseTest() { } @Test - fun `disable-time-check-disabled feature can be set via test settings`() { + fun `disable-time-check-disabled feature can not be set via test settings`() { val rawConfig = ApplicationConfigurationAndroid.newBuilder() .setAppFeatures( AppFeatures.newBuilder().apply { @@ -169,22 +161,11 @@ class CWAConfigMapperTest : BaseTest() { .build() every { CWADebug.isDeviceForTestersBuild } returns false - every { testSettings.isDeviceTimeCheckDisabled } returns mockFlowPreference(false) - createInstance().map(rawConfig).apply { - isDeviceTimeCheckEnabled shouldBe true - } - - every { testSettings.isDeviceTimeCheckDisabled } returns mockFlowPreference(true) createInstance().map(rawConfig).apply { isDeviceTimeCheckEnabled shouldBe true } every { CWADebug.isDeviceForTestersBuild } returns true - createInstance().map(rawConfig).apply { - isDeviceTimeCheckEnabled shouldBe false - } - - every { testSettings.isDeviceTimeCheckDisabled } returns mockFlowPreference(false) createInstance().map(rawConfig).apply { isDeviceTimeCheckEnabled shouldBe true } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt index cf940d648..0f8ffbf26 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt @@ -4,6 +4,8 @@ import de.rki.coronawarnapp.appconfig.download.AppConfigApiV2 import de.rki.coronawarnapp.appconfig.internal.ApplicationConfigurationCorruptException import de.rki.coronawarnapp.appconfig.internal.ApplicationConfigurationInvalidException import de.rki.coronawarnapp.appconfig.internal.InternalConfigData +import de.rki.coronawarnapp.storage.TestSettings +import de.rki.coronawarnapp.util.CWADebug import de.rki.coronawarnapp.util.TimeStamper import de.rki.coronawarnapp.util.security.VerificationKeys import io.kotest.assertions.throwables.shouldThrow @@ -13,6 +15,7 @@ import io.mockk.clearAllMocks import io.mockk.coEvery import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.mockkObject import io.mockk.verify import kotlinx.coroutines.test.runBlockingTest import okhttp3.Headers @@ -25,6 +28,7 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import retrofit2.Response import testhelpers.BaseIOTest +import testhelpers.preferences.mockFlowPreference import java.io.File class AppConfigServerTest : BaseIOTest() { @@ -32,6 +36,7 @@ class AppConfigServerTest : BaseIOTest() { @MockK lateinit var api: AppConfigApiV2 @MockK lateinit var verificationKeys: VerificationKeys @MockK lateinit var timeStamper: TimeStamper + @MockK lateinit var testSettings: TestSettings private val testDir = File(IO_TEST_BASEDIR, this::class.simpleName!!) @BeforeEach @@ -42,6 +47,10 @@ class AppConfigServerTest : BaseIOTest() { every { timeStamper.nowUTC } returns Instant.ofEpochMilli(123456789) every { verificationKeys.hasInvalidSignature(any(), any()) } returns false + + mockkObject(CWADebug) + every { CWADebug.isDeviceForTestersBuild } returns false + every { testSettings.fakeCorrectDeviceTime } returns mockFlowPreference(false) } @AfterEach @@ -53,7 +62,8 @@ class AppConfigServerTest : BaseIOTest() { private fun createInstance() = AppConfigServer( api = { api }, verificationKeys = verificationKeys, - timeStamper = timeStamper + timeStamper = timeStamper, + testSettings = testSettings ) @Test @@ -167,6 +177,50 @@ class AppConfigServerTest : BaseIOTest() { ) } + @Test + fun `test setting can override device time offset on tester builds`() = runBlockingTest { + coEvery { api.getApplicationConfiguration() } returns Response.success( + APPCONFIG_BUNDLE.toResponseBody(), + Headers.headersOf( + "Date", "Tue, 03 Nov 2020 06:35:16 GMT", + "ETag", "I am an ETag :)!" + ) + ) + every { timeStamper.nowUTC } returns Instant.parse("2020-11-03T05:35:16.000Z") + + every { CWADebug.isDeviceForTestersBuild } returns true + every { testSettings.fakeCorrectDeviceTime } returns mockFlowPreference(true) + createInstance().downloadAppConfig() shouldBe InternalConfigData( + rawData = APPCONFIG_RAW, + serverTime = Instant.parse("2020-11-03T06:35:16.000Z"), + localOffset = Duration.standardMinutes(90), + etag = "I am an ETag :)!", + cacheValidity = Duration.standardSeconds(300) + ) + } + + @Test + fun `test setting can not override device time offset on prod builds`() = runBlockingTest { + coEvery { api.getApplicationConfiguration() } returns Response.success( + APPCONFIG_BUNDLE.toResponseBody(), + Headers.headersOf( + "Date", "Tue, 03 Nov 2020 06:35:16 GMT", + "ETag", "I am an ETag :)!" + ) + ) + every { timeStamper.nowUTC } returns Instant.parse("2020-11-03T05:35:16.000Z") + + every { CWADebug.isDeviceForTestersBuild } returns false + every { testSettings.fakeCorrectDeviceTime } returns mockFlowPreference(true) + createInstance().downloadAppConfig() shouldBe InternalConfigData( + rawData = APPCONFIG_RAW, + serverTime = Instant.parse("2020-11-03T06:35:16.000Z"), + localOffset = Duration.standardHours(-1), + etag = "I am an ETag :)!", + cacheValidity = Duration.standardSeconds(300) + ) + } + companion object { private val APPCONFIG_BUNDLE = ( -- GitLab