QR Code stricter validation (EXPOSUREAPP-2570) (#1127)

* test for OnboardingFragment

* QR code stricter validations

* regex pattern improvement

* removed Obsolete test

* lint changes to fix pipeline issue

* logic refinement and test coverage improvement

* pipeline related fixes

* piepline related fixes : unnecessary test removed

* pipeline fixes and increased tests coverage

* QR code validation logic improvement

* detekt pipeline issue resolved

* pipeline issue Fix

* logic ovehaul and improvement for QRCode scans / validations

1, Data object for ScanResults
2. new Test for ScanResults
3. logic simplified

* removing unused imports from tests

* Small refactoring together with Rituraj.

* Adjust detekt, early abort via return is GOOD!

Co-authored-by: Matthias Urhahn <darken@darken.eu>
Co-authored-by: Matthias Urhahn <matthias.urhahn@sap.com>

Corona-Warn-App/config/detekt.yml

@@ -572,7 +572,7 @@ style:
     excludedFunctions: 'equals'
     excludeLabeled: false
     excludeReturnFromLambda: true
-    excludeGuardClauses: false
+    excludeGuardClauses: true
   SafeCast:
     active: true
   SerialVersionUIDInSerializableClass:

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/QRScanResult.kt

+package de.rki.coronawarnapp.service.submission
+
+import java.util.regex.Pattern
+
+data class QRScanResult(val rawResult: String) {
+
+    val isValid: Boolean
+        get() = guid != null
+    val guid: String? by lazy { extractGUID(rawResult) }
+
+    private fun extractGUID(rawResult: String): String? {
+        if (rawResult.length > MAX_QR_CODE_LENGTH) return null
+        if (rawResult.count { it == GUID_SEPARATOR } != 1) return null
+        if (!QR_CODE_REGEX.toRegex().matches(rawResult)) return null
+
+        val potentialGUID = rawResult.substringAfterLast(GUID_SEPARATOR, "")
+        if (potentialGUID.isBlank() || potentialGUID.length > MAX_GUID_LENGTH) return null
+
+        return potentialGUID
+    }
+
+    companion object {
+        // regex pattern for scanned QR code URL
+        val QR_CODE_REGEX: Pattern = Pattern.compile(
+            "^((^https:\\/{2}localhost)(\\/\\?)[A-Fa-f0-9]{6}" +
+                    "[-][A-Fa-f0-9]{8}[-][A-Fa-f0-9]{4}[-][A-Fa-f0-9]{4}[-][A-Fa-f0-9]{4}[-][A-Fa-f0-9]{12})\$"
+        )
+        const val GUID_SEPARATOR = '?'
+        const val MAX_QR_CODE_LENGTH = 150
+        const val MAX_GUID_LENGTH = 80
+    }
+}

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/SubmissionConstants.kt

@@ -14,10 +14,6 @@ object SubmissionConstants {
     val TEST_RESULT_URL = "$VERSIONED_VERIFICATION_CDN_URL/$TEST_RESULT"
     val TAN_REQUEST_URL = "$VERSIONED_VERIFICATION_CDN_URL/$TAN"
 
-    const val MAX_QR_CODE_LENGTH = 150
-    const val MAX_GUID_LENGTH = 80
-    const val GUID_SEPARATOR = '?'
-
     const val SERVER_ERROR_CODE_400 = 400
 
     const val EMPTY_HEADER = ""

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/SubmissionService.kt

@@ -65,19 +65,10 @@ object SubmissionService {
     }
 
     fun containsValidGUID(scanResult: String): Boolean {
-        if (scanResult.length > SubmissionConstants.MAX_QR_CODE_LENGTH ||
-            scanResult.count { it == SubmissionConstants.GUID_SEPARATOR } != 1
-        )
-            return false
-
-        val potentialGUID = extractGUID(scanResult)
-
-        return !(potentialGUID.isEmpty() || potentialGUID.length > SubmissionConstants.MAX_GUID_LENGTH)
+        val scanResult = QRScanResult(scanResult)
+        return scanResult.isValid
     }
 
-    fun extractGUID(scanResult: String): String =
-        scanResult.substringAfterLast(SubmissionConstants.GUID_SEPARATOR, "")
-
     fun storeTestGUID(guid: String) = LocalData.testGUID(guid)
 
     fun deleteTestGUID() {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt

@@ -9,6 +9,7 @@ import de.rki.coronawarnapp.exception.ExceptionCategory
 import de.rki.coronawarnapp.exception.TransactionException
 import de.rki.coronawarnapp.exception.http.CwaWebException
 import de.rki.coronawarnapp.exception.reporting.report
+import de.rki.coronawarnapp.service.submission.QRScanResult
 import de.rki.coronawarnapp.service.submission.SubmissionService
 import de.rki.coronawarnapp.storage.LocalData
 import de.rki.coronawarnapp.storage.SubmissionRepository
@@ -98,10 +99,10 @@ class SubmissionViewModel : ViewModel() {
             _uiStateError
         )
 
-    fun validateAndStoreTestGUID(scanResult: String) {
-        if (SubmissionService.containsValidGUID(scanResult)) {
-            val guid = SubmissionService.extractGUID(scanResult)
-            SubmissionService.storeTestGUID(guid)
+    fun validateAndStoreTestGUID(rawResult: String) {
+        val scanResult = QRScanResult(rawResult)
+        if (scanResult.isValid) {
+            SubmissionService.storeTestGUID(scanResult.guid!!)
             _scanStatus.value = Event(ScanStatus.SUCCESS)
         } else {
             _scanStatus.value = Event(ScanStatus.INVALID)

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/ScanResultTest.kt

+package de.rki.coronawarnapp.service.submission
+
+import io.kotest.matchers.shouldBe
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import io.mockk.mockkObject
+import org.junit.Before
+import org.junit.Test
+
+class ScanResultTest {
+    private val guid = "123456-12345678-1234-4DA7-B166-B86D85475064"
+
+    @MockK
+    private lateinit var scanResult: QRScanResult
+
+    @Before
+    fun setUp() {
+        MockKAnnotations.init(this)
+        mockkObject(scanResult)
+        every { scanResult.isValid } returns false
+    }
+
+    @Test
+    fun containsValidGUID() {
+        //valid test
+        scanResult = QRScanResult("https://localhost/?$guid")
+        scanResult.isValid shouldBe true
+
+        // more invalid tests checks
+        scanResult = QRScanResult("http://localhost/?$guid")
+        scanResult.isValid shouldBe false
+        scanResult = QRScanResult("https://localhost/?")
+        scanResult.isValid shouldBe false
+        scanResult = QRScanResult("htps://wrongformat.com")
+        scanResult.isValid shouldBe false
+        scanResult =
+            QRScanResult("https://localhost/%20?3D6D08-3567F3F2-4DCF-43A3-8737-4CD1F87D6FDA")
+        scanResult.isValid shouldBe false
+        scanResult =
+            QRScanResult("https://some-host.com/?3D6D08-3567F3F2-4DCF-43A3-8737-4CD1F87D6FDA")
+        scanResult.isValid shouldBe false
+        scanResult = QRScanResult("https://localhost/?3567F3F2-4DCF-43A3-8737-4CD1F87D6FDA")
+        scanResult.isValid shouldBe false
+        scanResult = QRScanResult("https://localhost/?4CD1F87D6FDA")
+        scanResult.isValid shouldBe false
+    }
+
+    @Test
+    fun extractGUID() {
+        QRScanResult("https://localhost/?$guid").guid shouldBe guid
+    }
+}
+

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/SubmissionConstantsTest.kt

@@ -11,13 +11,16 @@ class SubmissionConstantsTest {
         Assert.assertEquals(KeyType.GUID.name, "GUID")
         Assert.assertEquals(KeyType.TELETAN.name, "TELETAN")
 
-        Assert.assertEquals(SubmissionConstants.REGISTRATION_TOKEN_URL, "version/v1/registrationToken")
+        Assert.assertEquals(
+            SubmissionConstants.REGISTRATION_TOKEN_URL,
+            "version/v1/registrationToken"
+        )
         Assert.assertEquals(SubmissionConstants.TEST_RESULT_URL, "version/v1/testresult")
         Assert.assertEquals(SubmissionConstants.TAN_REQUEST_URL, "version/v1/tan")
 
-        Assert.assertEquals(SubmissionConstants.MAX_QR_CODE_LENGTH, 150)
-        Assert.assertEquals(SubmissionConstants.MAX_GUID_LENGTH, 80)
-        Assert.assertEquals(SubmissionConstants.GUID_SEPARATOR, '?')
+        Assert.assertEquals(QRScanResult.MAX_QR_CODE_LENGTH, 150)
+        Assert.assertEquals(QRScanResult.MAX_GUID_LENGTH, 80)
+        Assert.assertEquals(QRScanResult.GUID_SEPARATOR, '?')
 
         Assert.assertEquals(SubmissionConstants.SERVER_ERROR_CODE_400, 400)
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/SubmissionServiceTest.kt

@@ -163,26 +163,4 @@ class SubmissionServiceTest {
         }
     }
 
-    @Test
-    fun containsValidGUID() {
-        // valid
-        assertThat(
-            SubmissionService.containsValidGUID("https://bs-sd.de/covid-19/?$guid"),
-            equalTo(true)
-        )
-
-        // invalid
-        assertThat(
-            SubmissionService.containsValidGUID("https://no-guid-here"),
-            equalTo(false)
-        )
-    }
-
-    @Test
-    fun extractGUID() {
-        assertThat(
-            SubmissionService.extractGUID("https://bs-sd.de/covid-19/?$guid"),
-            equalTo(guid)
-        )
-    }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModelTest.kt

@@ -51,7 +51,7 @@ class SubmissionViewModelTest {
 
         // valid guid
         val guid = "123456-12345678-1234-4DA7-B166-B86D85475064"
-        viewModel.validateAndStoreTestGUID("https://bs-sd.de/covid-19/?$guid")
+        viewModel.validateAndStoreTestGUID("https://localhost/?$guid")
         viewModel.scanStatus.value?.getContent().let { Assert.assertEquals(ScanStatus.SUCCESS, it) }
 
         // invalid guid