[db] make repo certs non-null and remove repos without cert

historically, repos were added to the DB without much information and could stay in a broken state until manually removed. If a repo is updating, it should have a cert. So only repos that never did a single index update don't have a cert. Nowadays, this can not happen anymore as we get the repo and its cert before adding it to the DB. So whenever we update a repo, we know its certificate and fingerprint. Thus, this includes a DB migration removing all broken repos and making the certificate for repos in the DB non-null.
2026-02-16 01:53:16 -05:00 · 2024-04-05 18:04:52 -03:00
parent 5cf22c5e06
commit fa65084e60
26 changed files with 2373 additions and 168 deletions
--- a/libs/database/src/dbTest/java/org/fdroid/database/DbTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/DbTest.kt
@@ -76,9 +76,9 @@ internal abstract class DbTest {
        certificate: String = CERTIFICATE,
        lastTimestamp: Long = -1,
    ): Long {
-        val repoId = db.getRepositoryDao().insertEmptyRepo(address)
+        val repoId = db.getRepositoryDao().insertEmptyRepo(address, certificate = certificate)
        val streamReceiver = DbV1StreamReceiver(db, repoId) { true }
-        val indexProcessor = IndexV1StreamProcessor(streamReceiver, certificate, lastTimestamp)
+        val indexProcessor = IndexV1StreamProcessor(streamReceiver, lastTimestamp)
        db.runInTransaction {
            assets.open(indexAssetPath).use { indexStream ->
                indexProcessor.process(indexStream)
@@ -93,9 +93,9 @@ internal abstract class DbTest {
        version: Long = 42L,
        certificate: String = CERTIFICATE,
    ): Long {
-        val repoId = db.getRepositoryDao().insertEmptyRepo(address)
+        val repoId = db.getRepositoryDao().insertEmptyRepo(address, certificate = certificate)
        val streamReceiver = DbV2StreamReceiver(db, repoId) { true }
-        val indexProcessor = IndexV2FullStreamProcessor(streamReceiver, certificate)
+        val indexProcessor = IndexV2FullStreamProcessor(streamReceiver)
        db.runInTransaction {
            assets.open(indexAssetPath).use { indexStream ->
                indexProcessor.process(version, indexStream) {}
--- a/libs/database/src/dbTest/java/org/fdroid/database/IndexV1InsertTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/IndexV1InsertTest.kt
@@ -62,7 +62,7 @@ internal class IndexV1InsertTest : DbTest() {
    private fun streamIndex(path: String): Long {
        val repoId = db.getRepositoryDao().insertEmptyRepo("https://f-droid.org/repo")
        val streamReceiver = TestStreamReceiver(repoId)
-        val indexProcessor = IndexV1StreamProcessor(streamReceiver, null, -1)
+        val indexProcessor = IndexV1StreamProcessor(streamReceiver, -1)
        db.runInTransaction {
            assets.open(path).use { indexStream ->
                indexProcessor.process(indexStream)
@@ -80,7 +80,7 @@ internal class IndexV1InsertTest : DbTest() {
                val streamReceiver = TestStreamReceiver(repoId) {
                    if (cIn.byteCount > 0) throw SerializationException()
                }
-                val indexProcessor = IndexV1StreamProcessor(streamReceiver, null, -1)
+                val indexProcessor = IndexV1StreamProcessor(streamReceiver, -1)
                cIn.use { indexStream ->
                    indexProcessor.process(indexStream)
                }
@@ -100,8 +100,8 @@ internal class IndexV1InsertTest : DbTest() {
        private val callback: () -> Unit = {},
    ) : IndexV1StreamReceiver {
        private val streamReceiver = DbV1StreamReceiver(db, repoId) { true }
-        override fun receive(repo: RepoV2, version: Long, certificate: String?) {
-            streamReceiver.receive(repo, version, certificate)
+        override fun receive(repo: RepoV2, version: Long) {
+            streamReceiver.receive(repo, version)
            callback()
        }

--- a/libs/database/src/dbTest/java/org/fdroid/database/IndexV2InsertTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/IndexV2InsertTest.kt
@@ -64,7 +64,7 @@ internal class IndexV2InsertTest : DbTest() {
            db.runInTransaction {
                val repoId = db.getRepositoryDao().insertEmptyRepo("http://example.org")
                val streamReceiver = DbV2StreamReceiver(db, repoId, compatibilityChecker)
-                val indexProcessor = IndexV2FullStreamProcessor(streamReceiver, "")
+                val indexProcessor = IndexV2FullStreamProcessor(streamReceiver)
                cIn.use { indexStream ->
                    indexProcessor.process(42, indexStream) {}
                }
--- a/libs/database/src/dbTest/java/org/fdroid/database/MultiRepoMigrationTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/MultiRepoMigrationTest.kt
@@ -232,6 +232,7 @@ internal class MultiRepoMigrationTest {

        // now get the Room DB, so we can use our DAOs for verifying the migration
        databaseBuilder(getApplicationContext(), FDroidDatabaseInt::class.java, TEST_DB)
+            .addMigrations(MIGRATION_2_3)
            .allowMainThreadQueries()
            .build()
            .use { db ->
@@ -274,15 +275,11 @@ internal class MultiRepoMigrationTest {

        // now get the Room DB, so we can use our DAOs for verifying the migration
        databaseBuilder(getApplicationContext(), FDroidDatabaseInt::class.java, TEST_DB)
+            .addMigrations(MIGRATION_2_3)
            .allowMainThreadQueries()
            .build().use { db ->
-                // repo without cert did not get migrated
-                assertEquals(1, db.getRepositoryDao().getRepositories().size)
-                val repo = db.getRepositoryDao().getRepositories()[0]
-                // cert is still null
-                assertNull(repo.certificate)
-                // address still the same
-                assertEquals(fdroidRepo.address, repo.address)
+                // repo without cert did not get migrated, because we auto-migrate to latest version
+                assertEquals(0, db.getRepositoryDao().getRepositories().size)
            }
    }

@@ -314,6 +311,7 @@ internal class MultiRepoMigrationTest {

        // now get the Room DB, so we can use our DAOs for verifying the migration
        databaseBuilder(getApplicationContext(), FDroidDatabaseInt::class.java, TEST_DB)
+            .addMigrations(MIGRATION_2_3)
            .allowMainThreadQueries()
            .build().use { db ->
                check(db)
--- a/libs/database/src/dbTest/java/org/fdroid/database/RepoCertNonNullMigrationTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/RepoCertNonNullMigrationTest.kt
@@ -0,0 +1,87 @@
+package org.fdroid.database
+
+import android.content.ContentValues
+import android.database.sqlite.SQLiteDatabase
+import androidx.room.Room
+import androidx.room.testing.MigrationTestHelper
+import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
+import androidx.test.core.app.ApplicationProvider.getApplicationContext
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation
+import org.fdroid.database.Converters.localizedTextV2toString
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import kotlin.test.assertEquals
+
+private const val TEST_DB = "migration-test"
+
+@RunWith(AndroidJUnit4::class)
+internal class RepoCertNonNullMigrationTest {
+
+    @get:Rule
+    val helper: MigrationTestHelper = MigrationTestHelper(
+        instrumentation = getInstrumentation(),
+        databaseClass = FDroidDatabaseInt::class.java,
+        specs = emptyList(),
+        openFactory = FrameworkSQLiteOpenHelperFactory(),
+    )
+
+    @Test
+    fun migrateRepos() {
+        helper.createDatabase(TEST_DB, 2).use { db ->
+            // Database has schema version 2. Insert some data using SQL queries.
+            // We can't use DAO classes because they expect the latest schema.
+            val repoId1 = db.insert(
+                CoreRepository.TABLE,
+                SQLiteDatabase.CONFLICT_FAIL,
+                ContentValues().apply {
+                    put("name", localizedTextV2toString(mapOf("en-US" to "foo")))
+                    put("description", localizedTextV2toString(mapOf("en-US" to "bar")))
+                    put("address", "https://example.org/repo")
+                    put("certificate", "0123")
+                    put("timestamp", -1)
+                })
+            db.insert(
+                RepositoryPreferences.TABLE,
+                SQLiteDatabase.CONFLICT_FAIL,
+                ContentValues().apply {
+                    put("repoId", repoId1)
+                    put("enabled", true)
+                    put("weight", Long.MAX_VALUE)
+                })
+            val repoId2 = db.insert(
+                CoreRepository.TABLE,
+                SQLiteDatabase.CONFLICT_FAIL,
+                ContentValues().apply {
+                    put("name", localizedTextV2toString(mapOf("en-US" to "no cert")))
+                    put("description", localizedTextV2toString(mapOf("en-US" to "no cert desc")))
+                    put("address", "https://example.com/repo")
+                    put("timestamp", -1)
+                })
+            db.insert(
+                RepositoryPreferences.TABLE,
+                SQLiteDatabase.CONFLICT_FAIL,
+                ContentValues().apply {
+                    put("repoId", repoId2)
+                    put("enabled", true)
+                    put("weight", Long.MAX_VALUE - 2)
+                })
+        }
+
+        // Re-open the database with version 2, auto-migrations are applied automatically
+        helper.runMigrationsAndValidate(TEST_DB, 4, true, MIGRATION_2_3).close()
+
+        // now get the Room DB, so we can use our DAOs for verifying the migration
+        Room.databaseBuilder(getApplicationContext(), FDroidDatabaseInt::class.java, TEST_DB)
+            .addMigrations(MIGRATION_2_3)
+            .allowMainThreadQueries()
+            .build().use { db ->
+                // repo without cert did not get migrated, the other one did
+                assertEquals(1, db.getRepositoryDao().getRepositories().size)
+                val repo = db.getRepositoryDao().getRepositories()[0]
+                assertEquals("https://example.org/repo", repo.address)
+            }
+    }
+
+}
--- a/libs/database/src/dbTest/java/org/fdroid/database/RepositoryDaoTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/RepositoryDaoTest.kt
@@ -265,19 +265,6 @@ internal class RepositoryDaoTest : DbTest() {
        assertEquals(repositoryPreferences, repoDao.getRepositoryPreferences(repoId))
    }

-    @Test
-    fun certGetsUpdated() {
-        val repoId = repoDao.insertOrReplace(getRandomRepo())
-        assertEquals(1, repoDao.getRepositories().size)
-        assertEquals(null, repoDao.getRepositories()[0].certificate)
-
-        val cert = getRandomString()
-        repoDao.updateRepository(repoId, cert)
-
-        assertEquals(1, repoDao.getRepositories().size)
-        assertEquals(cert, repoDao.getRepositories()[0].certificate)
-    }
-
    @Test
    fun testGetMinRepositoryWeight() {
        assertEquals(Int.MAX_VALUE, repoDao.getMinRepositoryWeight())
@@ -344,13 +331,13 @@ internal class RepositoryDaoTest : DbTest() {
        )

        // we'll add an archive repo for repo1 to the list [3, 5, (1, 1a), 4, 2]
-        repoDao.updateRepository(repoId1, "1234abcd")
        val repo1 = repoDao.getRepository(repoId1) ?: fail()
+        repoDao.updateRepository(repo1.repository.copy(certificate = "1234abcd"))
        val repo1a = InitialRepository(
            name = getRandomString(),
            address = "https://example.org/archive",
            description = getRandomString(),
-            certificate = repo1.certificate ?: fail(),
+            certificate = "1234abcd", // same as repo1
            version = 42L,
            enabled = false,
        )
--- a/libs/database/src/dbTest/java/org/fdroid/index/IndexUpdaterTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/index/IndexUpdaterTest.kt
@@ -17,7 +17,7 @@ internal class IndexUpdaterTest {
            address = "http://example.org/",
            timestamp = 1337L,
            formatVersion = IndexFormatVersion.TWO,
-            certificate = null,
+            certificate = "abcd",
            version = 2001,
            weight = 0,
            lastUpdated = 23L,
--- a/libs/database/src/dbTest/java/org/fdroid/index/v1/IndexV1UpdaterTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/index/v1/IndexV1UpdaterTest.kt
@@ -57,10 +57,10 @@ internal class IndexV1UpdaterTest : DbTest() {

    @Test
    fun testIndexV1Processing() {
-        val repoId = repoDao.insertEmptyRepo(TESTY_CANONICAL_URL)
+        val repoId = repoDao.insertEmptyRepo(TESTY_CANONICAL_URL, certificate = TESTY_CERT)
        val repo = repoDao.getRepository(repoId) ?: fail()
        downloadIndex(repo, TESTY_JAR)
-        val result = indexUpdater.updateNewRepo(repo, TESTY_FINGERPRINT).noError()
+        val result = indexUpdater.update(repo).noError()
        assertIs<IndexUpdateResult.Processed>(result)

        // repo got updated
@@ -124,7 +124,7 @@ internal class IndexV1UpdaterTest : DbTest() {
        val repoId = repoDao.insertEmptyRepo(TESTY_CANONICAL_URL)
        val repo = repoDao.getRepository(repoId) ?: fail()
        downloadIndex(repo, TESTY_JAR)
-        val result = indexUpdater.updateNewRepo(repo, "not the right fingerprint")
+        val result = indexUpdater.update(repo)
        assertIs<IndexUpdateResult.Error>(result)
        assertIs<SigningException>(result.e)

@@ -141,7 +141,7 @@ internal class IndexV1UpdaterTest : DbTest() {
        val futureRepo =
            repo.copy(repository = repo.repository.copy(timestamp = System.currentTimeMillis()))
        downloadIndex(futureRepo, TESTY_JAR)
-        val result = indexUpdater.updateNewRepo(futureRepo, TESTY_FINGERPRINT)
+        val result = indexUpdater.update(futureRepo)
        assertIs<IndexUpdateResult.Error>(result)
        assertIs<OldIndexException>(result.e)
        assertFalse((result.e as OldIndexException).isSameTimestamp)
@@ -208,7 +208,7 @@ internal class IndexV1UpdaterTest : DbTest() {
        val repoId = repoDao.insertEmptyRepo("http://example.org")
        val repo = repoDao.getRepository(repoId) ?: fail()
        downloadIndex(repo, jar)
-        return indexUpdater.updateNewRepo(repo, null)
+        return indexUpdater.update(repo)
    }

    /**
--- a/libs/database/src/dbTest/java/org/fdroid/index/v2/IndexV2UpdaterTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/index/v2/IndexV2UpdaterTest.kt
@@ -21,15 +21,14 @@ import org.fdroid.test.TestDataMaxV2
 import org.fdroid.test.TestDataMidV2
 import org.fdroid.test.TestDataMinV2
 import org.fdroid.test.VerifierConstants.CERTIFICATE
-import org.fdroid.test.VerifierConstants.FINGERPRINT
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
 import org.junit.runner.RunWith
 import kotlin.test.assertEquals
+import kotlin.test.assertIs
 import kotlin.test.assertNull
-import kotlin.test.assertTrue
 import kotlin.test.fail

@RunWith(AndroidJUnit4::class)
@@ -57,18 +56,18 @@ internal class IndexV2UpdaterTest : DbTest() {

    @Test
    fun testFullIndexEmptyToMin() {
-        val repoId = repoDao.insertEmptyRepo("http://example.org")
+        val repoId = repoDao.insertEmptyRepo("http://example.org", certificate = CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-min/$SIGNED_FILE_NAME",
            jsonPath = "index-min-v2.json",
            indexFileV2 = TestDataEntry.emptyToMin.index
        )
-        val result = indexUpdater.updateNewRepo(repo, FINGERPRINT).noError()
+        val result = indexUpdater.update(repo).noError()
        assertEquals(IndexUpdateResult.Processed, result)
        assertDbEquals(repoId, TestDataMinV2.index)

-        // check that certificate and format version got entered
+        // check that format version got entered and certificate stayed the same
        val updatedRepo = repoDao.getRepository(repoId) ?: fail()
        assertEquals(TWO, updatedRepo.formatVersion)
        assertEquals(CERTIFICATE, updatedRepo.certificate)
@@ -77,14 +76,14 @@ internal class IndexV2UpdaterTest : DbTest() {

    @Test
    fun testFullIndexEmptyToMid() {
-        val repoId = repoDao.insertEmptyRepo("http://example.org")
+        val repoId = repoDao.insertEmptyRepo("http://example.org", certificate = CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-mid/$SIGNED_FILE_NAME",
            jsonPath = "index-mid-v2.json",
            indexFileV2 = TestDataEntry.emptyToMid.index
        )
-        val result = indexUpdater.updateNewRepo(repo, FINGERPRINT).noError()
+        val result = indexUpdater.update(repo).noError()
        assertEquals(IndexUpdateResult.Processed, result)
        assertDbEquals(repoId, TestDataMidV2.index)
        assertTimestampRecent(repoDao.getRepository(repoId)?.lastUpdated)
@@ -92,14 +91,14 @@ internal class IndexV2UpdaterTest : DbTest() {

    @Test
    fun testFullIndexEmptyToMax() {
-        val repoId = repoDao.insertEmptyRepo("http://example.org")
+        val repoId = repoDao.insertEmptyRepo("http://example.org", certificate = CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-max/$SIGNED_FILE_NAME",
            jsonPath = "index-max-v2.json",
            indexFileV2 = TestDataEntry.emptyToMax.index
        )
-        val result = indexUpdater.updateNewRepo(repo, FINGERPRINT).noError()
+        val result = indexUpdater.update(repo).noError()
        assertEquals(IndexUpdateResult.Processed, result)
        assertDbEquals(repoId, TestDataMaxV2.index)
        assertTimestampRecent(repoDao.getRepository(repoId)?.lastUpdated)
@@ -123,7 +122,6 @@ internal class IndexV2UpdaterTest : DbTest() {
    @Test
    fun testDiffEmptyToMin() {
        val repoId = streamIndexV2IntoDb("index-empty-v2.json")
-        repoDao.updateRepository(repoId, CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-min/$SIGNED_FILE_NAME",
@@ -139,7 +137,6 @@ internal class IndexV2UpdaterTest : DbTest() {
    @Test
    fun testDiffMidToMax() {
        val repoId = streamIndexV2IntoDb("index-mid-v2.json")
-        repoDao.updateRepository(repoId, CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-max/$SIGNED_FILE_NAME",
@@ -155,7 +152,6 @@ internal class IndexV2UpdaterTest : DbTest() {
    @Test
    fun testSameTimestampUnchanged() {
        val repoId = streamIndexV2IntoDb("index-min-v2.json")
-        repoDao.updateRepository(repoId, CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-min/$SIGNED_FILE_NAME",
@@ -171,7 +167,6 @@ internal class IndexV2UpdaterTest : DbTest() {
    @Test
    fun testHigherTimestampUnchanged() {
        val repoId = streamIndexV2IntoDb("index-mid-v2.json")
-        repoDao.updateRepository(repoId, CERTIFICATE)
        val repo = prepareUpdate(
            repoId = repoId,
            entryPath = "diff-empty-min/$SIGNED_FILE_NAME",
@@ -186,7 +181,6 @@ internal class IndexV2UpdaterTest : DbTest() {
    @Test
    fun testNoDiffFoundIndexFallback() {
        val repoId = streamIndexV2IntoDb("index-empty-v2.json")
-        repoDao.updateRepository(repoId, CERTIFICATE)
        // fake timestamp of internal repo, so we will fail to find a diff in entry.json
        val newRepo = repoDao.getRepository(repoId)?.repository?.copy(timestamp = 22) ?: fail()
        repoDao.updateRepository(newRepo)
@@ -203,20 +197,6 @@ internal class IndexV2UpdaterTest : DbTest() {

    @Test
    fun testWrongFingerprint() {
-        val repoId = repoDao.insertEmptyRepo("http://example.org")
-        val repo = prepareUpdate(
-            repoId = repoId,
-            entryPath = "diff-empty-min/$SIGNED_FILE_NAME",
-            jsonPath = "index-min-v2.json",
-            indexFileV2 = TestDataEntry.emptyToMin.index
-        )
-        val result = indexUpdater.updateNewRepo(repo, "wrong fingerprint")
-        assertTrue(result is IndexUpdateResult.Error)
-        assertTrue(result.e is SigningException)
-    }
-
-    @Test
-    fun testNormalUpdateOnRepoWithMissingFingerprint() {
        val repoId = repoDao.insertEmptyRepo("http://example.org")
        val repo = prepareUpdate(
            repoId = repoId,
@@ -225,8 +205,8 @@ internal class IndexV2UpdaterTest : DbTest() {
            indexFileV2 = TestDataEntry.emptyToMin.index
        )
        val result = indexUpdater.update(repo)
-        assertTrue(result is IndexUpdateResult.Error)
-        assertTrue(result.e is IllegalArgumentException)
+        assertIs<IndexUpdateResult.Error>(result)
+        assertIs<SigningException>(result.e)
    }

    /**