diff --git a/app/schemas/org.libremail.data.local.AccountDatabase/1.json b/app/schemas/org.libremail.data.local.AccountDatabase/1.json new file mode 100644 index 0000000..dd0f0fa --- /dev/null +++ b/app/schemas/org.libremail.data.local.AccountDatabase/1.json @@ -0,0 +1,234 @@ +{ + "formatVersion": 1, + "database": { + "version": 1, + "identityHash": "f2bbe80e572de72f50869b14aca4c4bb", + "entities": [ + { + "tableName": "accounts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, `displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, `imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, `smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "email", + "columnName": "email", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "authType", + "columnName": "authType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.host", + "columnName": "imap_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.port", + "columnName": "imap_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "imap.security", + "columnName": "imap_security", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.host", + "columnName": "smtp_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.port", + "columnName": "smtp_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "smtp.security", + "columnName": "smtp_security", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "credentials", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "encryptedSecret", + "columnName": "encryptedSecret", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + } + }, + { + "tableName": "account_settings", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `signature` TEXT NOT NULL, `signatureEnabled` INTEGER NOT NULL, `notificationsEnabled` INTEGER NOT NULL, `retentionCount` INTEGER, `retentionMonths` INTEGER, PRIMARY KEY(`accountId`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "signature", + "columnName": "signature", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "signatureEnabled", + "columnName": "signatureEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "notificationsEnabled", + "columnName": "notificationsEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "retentionCount", + "columnName": "retentionCount", + "affinity": "INTEGER" + }, + { + "fieldPath": "retentionMonths", + "columnName": "retentionMonths", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + }, + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "signatures", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `name` TEXT NOT NULL, `contentHtml` TEXT NOT NULL, `isDefault` INTEGER NOT NULL, PRIMARY KEY(`id`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "contentHtml", + "columnName": "contentHtml", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isDefault", + "columnName": "isDefault", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_signatures_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_signatures_accountId` ON `${TABLE_NAME}` (`accountId`)" + } + ], + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + } + ], + "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, 'f2bbe80e572de72f50869b14aca4c4bb')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/AccountDataMigratorTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDataMigratorTest.kt new file mode 100644 index 0000000..d610200 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDataMigratorTest.kt @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.room.Room +import androidx.room.testing.MigrationTestHelper +import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.local.entity.CredentialEntity + +/** + * The one-time move performed by [AccountDataMigrator] (issue #111): copying accounts / credentials / + * per-account settings / signatures out of the cache database into the plaintext [AccountDatabase]. + * + * Exercises the [AccountDataMigrator.copyAccountTables] core directly (the full [AccountDataMigrator] + * also resolves the passphrase and flips the done-flag, which need the real DataStore/Keystore). A v14 + * cache is built with [MigrationTestHelper] from the exported schema, so the copy runs against exactly + * the on-disk shape an upgrading user has. + */ +@RunWith(AndroidJUnit4::class) +class AccountDataMigratorTest { + + @get:Rule + val helper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + LibreMailDatabase::class.java, + emptyList(), + FrameworkSQLiteOpenHelperFactory(), + ) + + private val context = ApplicationProvider.getApplicationContext() + private val cacheName = "acct-migrator-cache-test.db" + private val accountsName = "acct-migrator-accounts-test.db" + private val cacheFile get() = context.getDatabasePath(cacheName) + private val accountsFile get() = context.getDatabasePath(accountsName) + + // 64 hex chars == a 32-byte SQLCipher passphrase. + private val passphrase = "0123456789abcdef".repeat(4) + + @Before + @After + fun clean() { + listOf(cacheName, accountsName).forEach { name -> + context.deleteDatabase(name) + context.getDatabasePath(name).parentFile + ?.listFiles { f -> f.name.startsWith(name) } + ?.forEach { it.delete() } + } + } + + /** Builds a v14 cache holding one fully-populated account plus a mail row. */ + private fun seedVersion14Cache() { + helper.createDatabase(cacheName, 14).apply { + execSQL( + "INSERT INTO accounts (id, email, displayName, authType, imap_host, imap_port, imap_security, " + + "smtp_host, smtp_port, smtp_security) VALUES ('acct', 'ada@example.org', 'Ada', " + + "'PASSWORD_IMAP', 'imap.example.org', 993, 'SSL_TLS', 'smtp.example.org', 465, 'SSL_TLS')", + ) + execSQL("INSERT INTO credentials (accountId, encryptedSecret) VALUES ('acct', 'sealed-secret')") + execSQL( + "INSERT INTO account_settings (accountId, signature, signatureEnabled, notificationsEnabled, " + + "retentionCount, retentionMonths) VALUES ('acct', 'Cheers', 1, 0, NULL, 6)", + ) + execSQL( + "INSERT INTO signatures (id, accountId, name, contentHtml, isDefault) " + + "VALUES ('sig-1', 'acct', 'Work', '

Regards

', 1)", + ) + execSQL( + "INSERT INTO messages (id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " + + "timestampMillis, isRead, isStarred, folder, inInbox, bodyFetched, uid) VALUES " + + "('acct:INBOX:1', 'acct', 'Ada', 'a@x', 'Hi', '', '', 0, 1, 0, 0, 'INBOX', 1, 0, 1)", + ) + close() + } + } + + private fun openAccountsDb(): AccountDatabase = + Room.databaseBuilder(context, AccountDatabase::class.java, accountsName).build() + + @Test + fun movesEveryAccountTableOutOfAPlaintextCache() = runBlocking { + seedVersion14Cache() + + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + // First open lets Room stamp its identity onto the migrator-created file; reopen so a real + // session's reads run against a fully Room-owned database. + openAccountsDb().apply { + assertEquals("Ada", accountDao().getById("acct")?.displayName) + close() + } + openAccountsDb().apply { + val account = accountDao().getById("acct") + assertEquals("ada@example.org", account?.email) + assertEquals(993, account?.imap?.port) + assertEquals("smtp.example.org", account?.smtp?.host) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + val settings = accountSettingsDao().get("acct") + assertEquals(false, settings?.signatureEnabled) + assertEquals(6, settings?.retentionMonths) + assertNull(settings?.retentionCount) + val signatures = signatureDao().observeForAccount("acct").first() + assertEquals(listOf("Work"), signatures.map { it.name }) + assertTrue("the default flag must round-trip", signatures.single().isDefault) + close() + } + } + + @Test + fun movesAccountsOutOfAnEncryptedCache() = runBlocking { + seedVersion14Cache() + // Turn the cache into the SQLCipher form an app-lock + encrypted-cache user has on disk. + DatabaseEncryption.ensureEncrypted(cacheFile, passphrase) + assertTrue("precondition: the source cache is encrypted", DatabaseEncryption.isEncrypted(cacheFile)) + + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = passphrase, accountsFile = accountsFile) + + openAccountsDb().apply { + assertNotNull(accountDao().getById("acct")) + close() + } + openAccountsDb().apply { + assertEquals("ada@example.org", accountDao().getById("acct")?.email) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + close() + } + } + + @Test + fun reRunningTheCopyIsIdempotentAndKeepsLaterEdits() = runBlocking { + seedVersion14Cache() + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + // Simulate the user editing an account AFTER the migration. + openAccountsDb().apply { + val edited = accountDao().getById("acct")!!.copy(displayName = "Ada Lovelace") + accountDao().upsert(edited) + close() + } + + // A re-run (e.g. after a mid-startup crash before the done-flag was set) must not clobber it. + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + openAccountsDb().apply { + assertEquals(1, accountDao().getAll().size) + assertEquals( + "INSERT OR IGNORE must not overwrite the post-migration edit", + "Ada Lovelace", + accountDao().getById("acct")?.displayName, + ) + close() + } + } + + @Test + fun accountsAndCredentialsSurviveACacheWipe() = runBlocking { + seedVersion14Cache() + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + // The "clear + re-sync" recovery wipes only the cache file; AccountDatabase is a separate file. + context.deleteDatabase(cacheName) + assertTrue("precondition: the cache file is gone", !cacheFile.exists()) + + openAccountsDb().apply { + assertNotNull("the account must outlive a cache wipe (issue #111)", accountDao().getById("acct")) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + // And it is still usable: a fresh credential can be written with no cache present. + credentialDao().upsert(CredentialEntity("acct", "rotated")) + assertEquals("rotated", credentialDao().getById("acct")?.encryptedSecret) + close() + } + } + + @Test + fun migratorDdlMatchesExportedAccountDatabaseSchema() { + val schema = JSONObject( + InstrumentationRegistry.getInstrumentation().context.assets + .open("org.libremail.data.local.AccountDatabase/1.json") + .bufferedReader().use { it.readText() }, + ).getJSONObject("database") + val entities = schema.getJSONArray("entities") + + var checkedIndex = false + for (i in 0 until entities.length()) { + val entity = entities.getJSONObject(i) + val table = entity.getString("tableName") + val expectedCreate = entity.getString("createSql").replace("\${TABLE_NAME}", "`$table`") + assertEquals( + "AccountDataMigrator DDL for `$table` must match the exported AccountDatabase schema", + expectedCreate, + AccountDataMigrator.CREATE_TABLE_SQL[table], + ) + if (entity.has("indices")) { + val indices = entity.getJSONArray("indices") + for (j in 0 until indices.length()) { + val index = indices.getJSONObject(j) + if (index.getString("name") == "index_signatures_accountId") { + assertEquals( + "AccountDataMigrator signatures index must match the exported schema", + index.getString("createSql").replace("\${TABLE_NAME}", "`$table`"), + AccountDataMigrator.SIGNATURES_INDEX_SQL, + ) + checkedIndex = true + } + } + } + } + assertEquals( + "every migrator table DDL must correspond to an exported entity", + AccountDataMigrator.CREATE_TABLE_SQL.keys, + (0 until entities.length()).map { entities.getJSONObject(it).getString("tableName") }.toSet(), + ) + assertTrue("the signatures index must be present in the exported schema", checkedIndex) + } +} diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/AccountDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDatabaseTest.kt new file mode 100644 index 0000000..8ddae89 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDatabaseTest.kt @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.AccountSettingsEntity +import org.libremail.data.local.entity.CredentialEntity +import org.libremail.data.local.entity.ServerConfigEmbedded +import org.libremail.data.local.entity.SignatureEntity + +/** + * Behavior of the non-auth [AccountDatabase] that holds accounts, credentials, per-account settings + * and signatures after they were moved out of the cache database (issue #111). Confirms the tables' + * foreign keys still cascade from the account, now that they live together in this database. + */ +@RunWith(AndroidJUnit4::class) +class AccountDatabaseTest { + + private lateinit var db: AccountDatabase + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() + } + + @After + fun tearDown() = db.close() + + private fun account(id: String = "acct") = AccountEntity( + id = id, + email = "a@example.org", + displayName = "A", + authType = "PASSWORD_IMAP", + imap = ServerConfigEmbedded("imap.example.org", 993, "SSL_TLS"), + smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"), + ) + + @Test + fun credentialRoundTripsAndIsIndependentOfTheAccountRow() = runBlocking { + db.accountDao().upsert(account()) + db.credentialDao().upsert(CredentialEntity("acct", "sealed-secret")) + + assertEquals("sealed-secret", db.credentialDao().getById("acct")?.encryptedSecret) + } + + @Test + fun accountSettingsRoundTripAndCascadeWithTheirAccount() = runBlocking { + db.accountDao().upsert(account()) + db.accountSettingsDao().upsert( + AccountSettingsEntity("acct", signature = "Hi", signatureEnabled = false, notificationsEnabled = false), + ) + assertEquals("Hi", db.accountSettingsDao().get("acct")?.signature) + + db.accountDao().deleteById("acct") + + assertNull("account_settings must cascade-delete with its account", db.accountSettingsDao().get("acct")) + } + + @Test + fun signaturesCascadeWithTheirAccount() = runBlocking { + db.accountDao().upsert(account()) + db.signatureDao().upsert(SignatureEntity("sig-1", "acct", "Work", "

Regards

", isDefault = true)) + assertEquals(1, db.signatureDao().observeForAccount("acct").first().size) + + db.accountDao().deleteById("acct") + + assertTrue( + "signatures must cascade-delete with their account", + db.signatureDao().observeForAccount("acct").first().isEmpty(), + ) + } +} diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt index b4ed6cb..5879ba1 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -9,22 +9,21 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import org.junit.After import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.libremail.data.local.entity.AccountEntity -import org.libremail.data.local.entity.AccountSettingsEntity import org.libremail.data.local.entity.AttachmentEntity import org.libremail.data.local.entity.FolderEntity import org.libremail.data.local.entity.MessageEntity -import org.libremail.data.local.entity.ServerConfigEmbedded /** * Schema-behavior tests on a fresh in-memory database at the current version. The migration DDL * itself is exercised by [MigrationTest], which replays the schema chain exported to app/schemas. * (Migrations from before v7 predate schema export, so they can't be replayed there.) + * + * Account/credential/settings/signature behavior moved to [AccountDatabaseTest] with those tables + * (issue #111). */ @RunWith(AndroidJUnit4::class) class LibreMailDatabaseTest { @@ -97,30 +96,6 @@ class LibreMailDatabaseTest { ) } - @Test - fun accountSettingsRoundTripAndCascadeWithTheirAccount() = runBlocking { - val accountDao = db.accountDao() - val settingsDao = db.accountSettingsDao() - accountDao.upsert( - AccountEntity( - id = "acct", - email = "a@example.org", - displayName = "A", - authType = "PASSWORD_IMAP", - imap = ServerConfigEmbedded("imap.example.org", 993, "SSL_TLS"), - smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"), - ), - ) - settingsDao.upsert( - AccountSettingsEntity("acct", signature = "Hi", signatureEnabled = false, notificationsEnabled = false), - ) - assertEquals("Hi", settingsDao.get("acct")?.signature) - - accountDao.deleteById("acct") - - assertNull("account_settings must cascade-delete with its account", settingsDao.get("acct")) - } - @Test fun searchRowsAreNotInboxAndAreCleared() = runBlocking { val messageDao = db.messageDao() diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt index 6716829..1d669db 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt @@ -133,6 +133,9 @@ class MigrationTest { open?.close() val stepDb = helper.runMigrationsAndValidate(TEST_DB, migration.endVersion, true, migration) stepDb.writeMidChainData() + // v16 moves the account tables out to AccountDatabase and drops them, so assert their rows + // and backfills reached v15 intact — just before the move (issue #111). + if (stepDb.version == 15) stepDb.assertAccountDataPresentAtV15() open = stepDb } val db = checkNotNull(open) { "no migration starts at v$OLDEST_EXPORTED_SCHEMA" } @@ -140,6 +143,29 @@ class MigrationTest { assertEquals("the chain must end at the newest exported schema", latestExportedSchemaVersion(), db.version) db.assertVersion7CacheSurvived() db.assertMigrationBackfillsApplied() + db.assertAccountTablesDroppedAtV16() + db.close() + } + + /** v15 -> v16 (issue #111): the moved account tables are dropped and the mail cache is untouched. */ + @Test + fun migrate15To16_dropsMovedAccountTablesAndKeepsCache() { + helper.createDatabase(TEST_DB, 15).apply { + insertAccount() + execSQL("INSERT INTO credentials (accountId, encryptedSecret) VALUES ('acct', 'sealed')") + execSQL( + "INSERT INTO messages (id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " + + "timestampMillis, isRead, isStarred, folder, inInbox, bodyFetched, uid) VALUES " + + "('acct:INBOX:1', 'acct', 'Ada', 'ada@example.org', 'Hi', '', '', 0, 1000, 0, 0, " + + "'INBOX', 1, 0, 1)", + ) + close() + } + + val db = helper.runMigrationsAndValidate(TEST_DB, 16, true, MIGRATION_15_16) + + db.assertAccountTablesDroppedAtV16() + assertEquals("the mail cache must be untouched by 15->16", 1, db.count("messages")) db.close() } @@ -203,9 +229,8 @@ class MigrationTest { } } - /** Every row cached at v7 must still be present and correct at the end of the chain. */ + /** Every mail-cache row cached at v7 must survive to v16 (account tables are checked separately). */ private fun SupportSQLiteDatabase.assertVersion7CacheSurvived() { - assertEquals(1, count("accounts")) assertEquals(2, count("messages")) assertEquals(1, count("outbox")) assertEquals(1, count("drafts")) @@ -215,24 +240,14 @@ class MigrationTest { assertEquals("Analytical engines", c.getString(1)) assertEquals(1, c.getInt(2)) } - query("SELECT encryptedSecret FROM credentials WHERE accountId = 'acct'").use { c -> - assertTrue("stored credentials must never be dropped by a migration", c.moveToFirst()) - assertEquals("sealed-secret", c.getString(0)) - } query("SELECT filename FROM attachments WHERE messageId = 'acct:1'").use { c -> assertTrue("attachment rows must survive the 6->7 style table rebuilds", c.moveToFirst()) assertEquals("notes.pdf", c.getString(0)) } } - /** Columns and rows created by the migrations themselves must hold their documented defaults. */ + /** Cache-table columns/rows the migrations backfill must hold their documented defaults at v16. */ private fun SupportSQLiteDatabase.assertMigrationBackfillsApplied() { - // 8->9 backfills one default settings row per existing account. - query("SELECT signatureEnabled, notificationsEnabled FROM account_settings").use { c -> - assertTrue("8->9 must backfill a settings row for the v7 account", c.moveToFirst()) - assertEquals(1, c.getInt(0)) - assertEquals(1, c.getInt(1)) - } // 9->10 adds bcc columns defaulting to ''; 10->11 adds nullable bodyHtml. query("SELECT bccAddresses, bodyHtml FROM outbox WHERE id = 'out-1'").use { c -> assertTrue(c.moveToFirst()) @@ -244,14 +259,6 @@ class MigrationTest { assertEquals("", c.getString(0)) assertTrue(c.isNull(1)) } - // 10->11 turns the signature written at v9 into that account's default rich-text signature. - query("SELECT name, contentHtml, isDefault FROM signatures WHERE accountId = 'acct'").use { c -> - assertTrue("10->11 must backfill the legacy per-account signature", c.moveToFirst()) - assertEquals("Signature", c.getString(0)) - assertEquals("Cheers,
Ada", c.getString(1)) - assertEquals(1, c.getInt(2)) - assertFalse("exactly one signature row must be backfilled", c.moveToNext()) - } // 11->12 stamps the folder cached at v8 as not special-use. query("SELECT specialUse FROM folders WHERE fullName = 'INBOX'").use { c -> assertTrue("folder cached at v8 must survive to the newest version", c.moveToFirst()) @@ -265,6 +272,42 @@ class MigrationTest { } } + /** + * The account tables' rows + migration backfills must be intact at v15, just before 15->16 moves + * them to [AccountDatabase] and drops them (issue #111). AccountDataMigrator's own copy is + * exercised in `AccountDataMigratorTest`; here we only assert the source rows reach the move point. + */ + private fun SupportSQLiteDatabase.assertAccountDataPresentAtV15() { + assertEquals(1, count("accounts")) + query("SELECT encryptedSecret FROM credentials WHERE accountId = 'acct'").use { c -> + assertTrue("stored credentials must reach v15 before the move", c.moveToFirst()) + assertEquals("sealed-secret", c.getString(0)) + } + // 8->9 backfills one default settings row per existing account. + query("SELECT signatureEnabled, notificationsEnabled FROM account_settings").use { c -> + assertTrue("8->9 must backfill a settings row for the v7 account", c.moveToFirst()) + assertEquals(1, c.getInt(0)) + assertEquals(1, c.getInt(1)) + } + // 10->11 turns the signature written at v9 into that account's default rich-text signature. + query("SELECT name, contentHtml, isDefault FROM signatures WHERE accountId = 'acct'").use { c -> + assertTrue("10->11 must backfill the legacy per-account signature", c.moveToFirst()) + assertEquals("Signature", c.getString(0)) + assertEquals("Cheers,
Ada", c.getString(1)) + assertEquals(1, c.getInt(2)) + assertFalse("exactly one signature row must be backfilled", c.moveToNext()) + } + } + + /** 15->16 drops the account tables from the cache (AccountDataMigrator copies them out first). */ + private fun SupportSQLiteDatabase.assertAccountTablesDroppedAtV16() { + listOf("accounts", "credentials", "account_settings", "signatures").forEach { table -> + query("SELECT name FROM sqlite_master WHERE type = 'table' AND name = '$table'").use { c -> + assertFalse("15->16 must drop `$table` from the cache database", c.moveToFirst()) + } + } + } + private fun SupportSQLiteDatabase.count(table: String): Int = query("SELECT COUNT(*) FROM $table").use { c -> c.moveToFirst() c.getInt(0) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt index 25355e6..bbd2bbf 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt @@ -26,7 +26,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.libremail.R import org.libremail.contacts.ContactsRepository -import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.AccountDatabase import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository import org.libremail.domain.model.Account @@ -56,7 +56,7 @@ class ComposeScreenTest { smtp = ServerConfig("smtp.example.com", 465, MailSecurity.SSL_TLS), ) - private var db: LibreMailDatabase? = null + private var db: AccountDatabase? = null private fun string(resId: Int) = composeTestRule.activity.getString(resId) @@ -79,7 +79,7 @@ class ComposeScreenTest { // Build the view model once and capture it, so recomposition doesn't recreate it. private fun setContent(mailRepository: FakeMailRepository = FakeMailRepository(), onBack: () -> Unit = {}) { val context = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext - val database = Room.inMemoryDatabaseBuilder(context, LibreMailDatabase::class.java).build().also { db = it } + val database = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build().also { db = it } val viewModel = ComposeViewModel( savedStateHandle = SavedStateHandle(), mailRepository = mailRepository, diff --git a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt index 20e1173..85f90c4 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt @@ -16,7 +16,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.libremail.R -import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.AccountDatabase import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository @@ -59,7 +59,7 @@ class AccountSettingsScreenTest { // stateIn/WhileSubscribed) keeps querying after the test body, so closing the in-memory DB out // from under it races and crashes ("connection pool has been closed"). The DB is reclaimed with // the test process. - val db = Room.inMemoryDatabaseBuilder(context, LibreMailDatabase::class.java).build() + val db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() val repository = AccountSettingsRepository(db.accountSettingsDao()) runBlocking { db.accountDao().upsert(account.toEntity()) // FK parent for the account_settings row diff --git a/app/src/main/kotlin/org/libremail/data/local/AccountDataMigrator.kt b/app/src/main/kotlin/org/libremail/data/local/AccountDataMigrator.kt new file mode 100644 index 0000000..8e5a50e --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/AccountDataMigrator.kt @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import android.util.Log +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.booleanPreferencesKey +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.preferencesDataStore +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.withContext +import net.zetetic.database.sqlcipher.SQLiteDatabase +import org.libremail.data.security.DatabaseKeyStore +import org.libremail.data.settings.SettingsRepository +import java.io.File +import javax.inject.Inject +import javax.inject.Singleton + +private val Context.accountMigrationDataStore: DataStore by + preferencesDataStore(name = "libremail_account_migration") + +/** + * One-time, crash-safe move of the account tables (`accounts`, `credentials`, `account_settings`, + * `signatures`) out of the auth-bound cache database [LibreMailDatabase] into the non-auth + * [AccountDatabase] (issue #111). Runs at startup, driven by `DatabaseModule.provideDatabase`, BEFORE + * Room opens the cache and its [MIGRATION_15_16] drops the moved tables. + * + * ### Why not a Room migration + * The copy is cross-database, so it needs `ATTACH DATABASE`, which SQLite forbids inside the + * transaction Room wraps every migration in. It therefore runs here on a dedicated SQLCipher + * connection before Room opens either database. + * + * ### Handling the encrypted source + * When the opt-in encrypted cache is on, the source `libremail.db` is SQLCipher-encrypted. The + * caller resolves and hands us its passphrase (the same one Room uses to open it); we attach the + * cache with that passphrase and copy into a plaintext `libremail-accounts.db`. When the cache is + * plaintext the passphrase is empty. Reading the source's schema validates the passphrase, so a + * genuinely wrong key fails loudly here (the same open would fail in Room) rather than losing data. + * + * The unrecoverable-key case does not reach us: `provideDatabase` wipes an undecryptable cache (and + * resets its seals) BEFORE calling us, so we then see a fresh/empty cache with nothing to move — the + * accounts trapped in that already-invalidated cache are lost regardless (the pre-existing bug), but + * no future invalidation can strand them again once they live in [AccountDatabase]. + * + * ### Crash-safety & idempotency + * - We never drop the source here; [MIGRATION_15_16] does that after we return, so if we crash the + * source rows are still intact for the next attempt. + * - The copy uses `INSERT OR IGNORE`, so a re-run after a mid-copy crash converges (existing rows + * are skipped, never duplicated, and never overwrite anything the user changed post-migration). + * - The "done" flag is only set after a successful copy; until then every start retries. Once set we + * return immediately and never touch the cache passphrase again — so after migration the account + * database opens with no Keystore dependency at all. + */ +@Singleton +class AccountDataMigrator @Inject constructor( + @ApplicationContext private val context: Context, + private val keyStore: DatabaseKeyStore, + private val settingsRepository: SettingsRepository, +) { + + /** + * Copy the account tables into [AccountDatabase] if it has not been done yet. Idempotent and + * safe to call from every `provideDatabase` construction. Throws (rather than silently skipping) + * on an unexpected copy failure so the caller does not proceed to drop the source tables — a + * crash-loop that preserves data is strictly safer than a wipe that loses it. + */ + suspend fun migrateIfNeeded() { + if (isDone()) return + val cacheFile = context.getDatabasePath(DatabaseFiles.NAME) + if (cacheFile.exists() && cacheFile.length() > 0L) { + // Read the cache in its CURRENT on-disk form. `provideDatabase` runs us before it converts + // between plaintext and encrypted, so the key is empty unless the file is encrypted now. + val cacheKey = if (DatabaseEncryption.isEncrypted(cacheFile)) { + keyStore.resolvePassphrase(settingsRepository.settings.first().appLock) + } else { + "" + } + val accountsFile = context.getDatabasePath(DatabaseFiles.ACCOUNTS_NAME) + withContext(Dispatchers.IO) { copyAccountTables(cacheFile, cacheKey, accountsFile) } + } + markDone() + } + + private suspend fun isDone(): Boolean = context.accountMigrationDataStore.data.first()[DONE] == true + + private suspend fun markDone() { + context.accountMigrationDataStore.edit { it[DONE] = true } + } + + companion object { + private const val TAG = "LibreMailAcctMigrate" + private val DONE = booleanPreferencesKey("accounts_moved_out_of_cache") + + /** The account tables, parent before children so foreign keys never block an insert. */ + private val TABLES = listOf("accounts", "credentials", "account_settings", "signatures") + + /** + * DDL for the account tables in [AccountDatabase] v1, copied verbatim from the exported Room + * schema (`schemas/org.libremail.data.local.AccountDatabase/1.json`). It MUST stay byte-for-byte + * identical to what Room generates for those entities, or Room silently accepts a subtly wrong + * schema (its identity check only compares the hash it writes, not the pre-existing tables). + * `AccountDataMigratorTest.migratorDdlMatchesExportedAccountDatabaseSchema` guards it against the + * exported schema; `internal` only so that test can read it. + */ + internal val CREATE_TABLE_SQL = mapOf( + "accounts" to + "CREATE TABLE IF NOT EXISTS `accounts` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, " + + "`displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, " + + "`imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, " + + "`smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))", + "credentials" to + "CREATE TABLE IF NOT EXISTS `credentials` (`accountId` TEXT NOT NULL, " + + "`encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))", + "account_settings" to + "CREATE TABLE IF NOT EXISTS `account_settings` (`accountId` TEXT NOT NULL, " + + "`signature` TEXT NOT NULL, `signatureEnabled` INTEGER NOT NULL, " + + "`notificationsEnabled` INTEGER NOT NULL, `retentionCount` INTEGER, " + + "`retentionMonths` INTEGER, PRIMARY KEY(`accountId`), " + + "FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) " + + "ON UPDATE NO ACTION ON DELETE CASCADE )", + "signatures" to + "CREATE TABLE IF NOT EXISTS `signatures` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, " + + "`name` TEXT NOT NULL, `contentHtml` TEXT NOT NULL, `isDefault` INTEGER NOT NULL, " + + "PRIMARY KEY(`id`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) " + + "ON UPDATE NO ACTION ON DELETE CASCADE )", + ) + + internal const val SIGNATURES_INDEX_SQL = + "CREATE INDEX IF NOT EXISTS `index_signatures_accountId` ON `signatures` (`accountId`)" + + /** + * Copies the account tables from [cacheFile] (opened with [cachePassphrase]; empty = plaintext) + * into a plaintext [accountsFile], creating the destination schema first. Opens the destination + * as `main` and attaches the (possibly encrypted) cache as `cache`, so a plaintext connection + * can still read the encrypted source via SQLCipher's per-attach key. Visible for the migrator + * test; call [migrateIfNeeded] in production. + */ + internal fun copyAccountTables(cacheFile: File, cachePassphrase: String, accountsFile: File) { + DatabaseEncryption.ensureNativeLibraryLoaded() + val db = SQLiteDatabase.openOrCreateDatabase( + accountsFile.absolutePath, + "".toByteArray(Charsets.US_ASCII), // destination is plaintext + null, + null, + ) + try { + // No WAL: keep the destination in rollback-journal mode (as DatabaseEncryption does) + // so that after close there is no -wal/-shm holding uncommitted rows for Room to miss. + db.rawExecSQL("PRAGMA journal_mode = DELETE;") + val keyLiteral = cachePassphrase.replace("'", "''") + val cachePath = cacheFile.absolutePath.replace("'", "''") + db.rawExecSQL("ATTACH DATABASE '$cachePath' AS cache KEY '$keyLiteral';") + try { + val present = presentTables(db) + if (present.isEmpty()) return // fresh cache or already dropped: nothing to move + TABLES.forEach { db.rawExecSQL(CREATE_TABLE_SQL.getValue(it)) } + db.rawExecSQL(SIGNATURES_INDEX_SQL) + // Parent first so an enforced foreign key (Room enables them; this raw connection + // does not) would still be satisfied. INSERT OR IGNORE makes each copy idempotent. + TABLES.filter { it in present }.forEach { table -> + db.rawExecSQL("INSERT OR IGNORE INTO `$table` SELECT * FROM cache.`$table`") + } + Log.d(TAG, "moved account tables into the account database: $present") + } finally { + db.rawExecSQL("DETACH DATABASE cache;") + } + } finally { + db.close() + } + // Room opens the destination next; drop any sidecars the copy left so a stale WAL/SHM can't + // confuse its first open. + val dir = accountsFile.parentFile + if (dir != null) { + listOf("-wal", "-shm", "-journal").forEach { File(dir, accountsFile.name + it).delete() } + } + } + + private fun presentTables(db: SQLiteDatabase): Set { + val names = TABLES.joinToString(",") { "'$it'" } + val present = mutableSetOf() + db.rawQuery( + "SELECT name FROM cache.sqlite_master WHERE type = 'table' AND name IN ($names)", + null, + ).use { cursor -> + while (cursor.moveToNext()) present += cursor.getString(0) + } + return present + } + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/AccountDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/AccountDatabase.kt new file mode 100644 index 0000000..cdf4e47 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/AccountDatabase.kt @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import androidx.room.Database +import androidx.room.RoomDatabase +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.AccountSettingsDao +import org.libremail.data.local.dao.CredentialDao +import org.libremail.data.local.dao.SignatureDao +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.AccountSettingsEntity +import org.libremail.data.local.entity.CredentialEntity +import org.libremail.data.local.entity.SignatureEntity + +/** + * Durable store for the pieces of an account that must survive a mail-cache wipe (issue #111): the + * account itself, its sealed credential, per-account settings, and saved signatures. + * + * This lives in its OWN database file ([DatabaseFiles.ACCOUNTS_NAME]) that is deliberately NEVER + * bound to the auth-bound SQLCipher key. When app-lock + encrypted-cache are on and that key is + * invalidated (a genuine biometric re-enrollment or lock removal/re-add), only the mail cache + * ([LibreMailDatabase]) becomes undecryptable and is wiped; this database is untouched, so the user + * stays signed in instead of being dropped back into onboarding. + * + * It is plaintext on disk. The only secret it holds is [CredentialEntity.encryptedSecret], which is + * already AES-GCM ciphertext sealed at the column level by the non-auth + * [org.libremail.data.security.KeystoreCrypto] master key (and that key survives an auth-key + * invalidation), so the secret never touches disk in the clear regardless of this file's own + * encryption. Account metadata (email address, server hosts) is not a secret. Keeping the file + * plaintext is what makes it maximally resilient — it can always be opened without any Keystore key, + * so no key invalidation can ever strand it. + * + * Existing installs are migrated into this database once, at startup, by [AccountDataMigrator] + * before [MIGRATION_15_16] drops the moved tables from the cache database. + */ +@Database( + entities = [ + AccountEntity::class, + CredentialEntity::class, + AccountSettingsEntity::class, + SignatureEntity::class, + ], + version = 1, + exportSchema = true, +) +abstract class AccountDatabase : RoomDatabase() { + abstract fun accountDao(): AccountDao + abstract fun credentialDao(): CredentialDao + abstract fun accountSettingsDao(): AccountSettingsDao + abstract fun signatureDao(): SignatureDao +} diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt index 515944c..6694a2c 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt @@ -40,7 +40,7 @@ object DatabaseEncryption { * tables but not that pragma, and a reset version would make Room attempt a bogus migration. */ private fun migrate(dbFile: File, sourcePassphrase: String, targetPassphrase: String) { - ensureLibraryLoaded() + ensureNativeLibraryLoaded() val dir = dbFile.parentFile ?: error("database file has no parent directory") val tmp = File(dir, dbFile.name + ".migrate").apply { delete() } @@ -98,7 +98,13 @@ object DatabaseEncryption { } @Volatile private var libraryLoaded = false - private fun ensureLibraryLoaded() { + + /** + * Load SQLCipher's native library once. Public so other startup helpers that open a database via + * [net.zetetic.database.sqlcipher.SQLiteDatabase] before Room does (e.g. [AccountDataMigrator]) + * can guarantee it is loaded first. + */ + fun ensureNativeLibraryLoaded() { if (libraryLoaded) return synchronized(this) { if (!libraryLoaded) { diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt index 0b18f42..4e52837 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt @@ -9,6 +9,13 @@ object DatabaseFiles { const val NAME = "libremail.db" + /** + * The [org.libremail.data.local.AccountDatabase] file — accounts, credentials, per-account + * settings and signatures. Deliberately a DIFFERENT file from [NAME] and NEVER wiped by [clear], + * so a cache-key invalidation keeps the user signed in (issue #111). + */ + const val ACCOUNTS_NAME = "libremail-accounts.db" + /** * Delete the database and any WAL/SHM/journal sidecars. Call only when no connection is open — * used by the "clear + re-sync" path when the encryption key is invalidated and the encrypted diff --git a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt index 24bf856..567ad08 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -3,52 +3,46 @@ package org.libremail.data.local import androidx.room.Database import androidx.room.RoomDatabase -import org.libremail.data.local.dao.AccountDao -import org.libremail.data.local.dao.AccountSettingsDao import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.BackfillProgressDao -import org.libremail.data.local.dao.CredentialDao import org.libremail.data.local.dao.DraftDao import org.libremail.data.local.dao.FolderDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.dao.OutboxDao -import org.libremail.data.local.dao.SignatureDao -import org.libremail.data.local.entity.AccountEntity -import org.libremail.data.local.entity.AccountSettingsEntity import org.libremail.data.local.entity.AttachmentEntity import org.libremail.data.local.entity.BackfillProgressEntity -import org.libremail.data.local.entity.CredentialEntity import org.libremail.data.local.entity.DraftEntity import org.libremail.data.local.entity.FolderEntity import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.entity.OutboxEntity -import org.libremail.data.local.entity.SignatureEntity +/** + * The offline mail cache. Everything here is re-derivable from the server on a fresh sync, so it is + * the database that opt-in SQLCipher encryption is applied to and — when the auth-bound key is + * invalidated — the one that "clear + re-sync" wipes. + * + * Account identity and user configuration (accounts, credentials, per-account settings, signatures) + * are deliberately NOT here: they live in [AccountDatabase], a separate non-auth-bound file, so a + * cache-key invalidation can never sign the user out (issue #111). [MIGRATION_15_16] dropped those + * tables from this database; [AccountDataMigrator] copies existing rows into [AccountDatabase] first. + */ @Database( entities = [ - AccountEntity::class, - AccountSettingsEntity::class, MessageEntity::class, - CredentialEntity::class, AttachmentEntity::class, OutboxEntity::class, DraftEntity::class, FolderEntity::class, - SignatureEntity::class, BackfillProgressEntity::class, ], - version = 15, + version = 16, exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { abstract fun messageDao(): MessageDao - abstract fun accountDao(): AccountDao - abstract fun accountSettingsDao(): AccountSettingsDao - abstract fun credentialDao(): CredentialDao abstract fun attachmentDao(): AttachmentDao abstract fun outboxDao(): OutboxDao abstract fun draftDao(): DraftDao abstract fun folderDao(): FolderDao - abstract fun signatureDao(): SignatureDao abstract fun backfillProgressDao(): BackfillProgressDao } diff --git a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt index 1b3a5f5..feadf8a 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -308,3 +308,25 @@ val MIGRATION_14_15 = object : Migration(14, 15) { db.execSQL("ALTER TABLE `folders` ADD COLUMN `hierarchyDelimiter` TEXT") } } + +/** + * v15 -> v16: move account identity + configuration OUT of the cache database (issue #111). The + * `accounts`, `credentials`, `account_settings` and `signatures` tables now live in [AccountDatabase] + * — a separate file that is never sealed by the auth-bound SQLCipher key — so a cache-key invalidation + * (biometric re-enrollment / lock removal) wipes only mail and can no longer sign the user out. + * + * The rows are copied into [AccountDatabase] by [AccountDataMigrator] at startup BEFORE Room opens the + * cache and runs this migration. The copy CANNOT happen here: Room wraps each migration in a + * transaction and SQLite forbids `ATTACH DATABASE` inside one, so a cross-database copy has to run on + * a separate connection before the cache is opened. This migration therefore only drops the tables + * that were moved. `DROP TABLE IF EXISTS` keeps it idempotent, and children (foreign-keyed to + * `accounts`) are dropped before the parent so the drop never trips a foreign-key check. + */ +val MIGRATION_15_16 = object : Migration(15, 16) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("DROP TABLE IF EXISTS `signatures`") + db.execSQL("DROP TABLE IF EXISTS `account_settings`") + db.execSQL("DROP TABLE IF EXISTS `credentials`") + db.execSQL("DROP TABLE IF EXISTS `accounts`") + } +} diff --git a/app/src/main/kotlin/org/libremail/di/AccountDatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/AccountDatabaseModule.kt new file mode 100644 index 0000000..435f929 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/di/AccountDatabaseModule.kt @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.di + +import android.content.Context +import androidx.room.Room +import dagger.Module +import dagger.Provides +import dagger.hilt.InstallIn +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.components.SingletonComponent +import org.libremail.data.local.AccountDatabase +import org.libremail.data.local.DatabaseFiles.ACCOUNTS_NAME +import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.AccountSettingsDao +import org.libremail.data.local.dao.CredentialDao +import org.libremail.data.local.dao.SignatureDao +import javax.inject.Singleton + +/** + * Hilt wiring for [AccountDatabase] — the non-auth-bound store for accounts, credentials, per-account + * settings and signatures (issue #111). Kept separate from [DatabaseModule] so each database's + * provides stay cohesive (and neither module grows past detekt's per-object function limit). + */ +@Module +@InstallIn(SingletonComponent::class) +object AccountDatabaseModule { + + /** + * The plaintext account store. Depends on [LibreMailDatabase] purely for construction ordering: + * building the cache runs the one-time [org.libremail.data.local.AccountDataMigrator] (which + * populates this file on a dedicated connection) and then drops the moved tables, so by the time + * Room opens this file the data is already present and no other connection is touching it. + */ + @Provides + @Singleton + fun provideAccountDatabase( + @ApplicationContext context: Context, + @Suppress("UNUSED_PARAMETER") cacheDatabase: LibreMailDatabase, + ): AccountDatabase = Room.databaseBuilder(context, AccountDatabase::class.java, ACCOUNTS_NAME).build() + + @Provides + fun provideAccountDao(database: AccountDatabase): AccountDao = database.accountDao() + + @Provides + fun provideCredentialDao(database: AccountDatabase): CredentialDao = database.credentialDao() + + @Provides + fun provideAccountSettingsDao(database: AccountDatabase): AccountSettingsDao = database.accountSettingsDao() + + @Provides + fun provideSignatureDao(database: AccountDatabase): SignatureDao = database.signatureDao() +} diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index 792e3ef..d247ad1 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -11,6 +11,7 @@ import dagger.hilt.components.SingletonComponent import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import net.zetetic.database.sqlcipher.SupportOpenHelperFactory +import org.libremail.data.local.AccountDataMigrator import org.libremail.data.local.DatabaseEncryption import org.libremail.data.local.DatabaseFiles import org.libremail.data.local.LibreMailDatabase @@ -19,6 +20,7 @@ import org.libremail.data.local.MIGRATION_11_12 import org.libremail.data.local.MIGRATION_12_13 import org.libremail.data.local.MIGRATION_13_14 import org.libremail.data.local.MIGRATION_14_15 +import org.libremail.data.local.MIGRATION_15_16 import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 @@ -28,16 +30,12 @@ import org.libremail.data.local.MIGRATION_6_7 import org.libremail.data.local.MIGRATION_7_8 import org.libremail.data.local.MIGRATION_8_9 import org.libremail.data.local.MIGRATION_9_10 -import org.libremail.data.local.dao.AccountDao -import org.libremail.data.local.dao.AccountSettingsDao import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.BackfillProgressDao -import org.libremail.data.local.dao.CredentialDao import org.libremail.data.local.dao.DraftDao import org.libremail.data.local.dao.FolderDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.dao.OutboxDao -import org.libremail.data.local.dao.SignatureDao import org.libremail.data.security.DatabaseKeyStore import org.libremail.data.settings.SettingsRepository import javax.inject.Singleton @@ -52,6 +50,7 @@ object DatabaseModule { @ApplicationContext context: Context, keyStore: DatabaseKeyStore, settingsRepository: SettingsRepository, + accountDataMigrator: AccountDataMigrator, ): LibreMailDatabase { val builder = Room.databaseBuilder(context, LibreMailDatabase::class.java, DB_NAME) .addMigrations( @@ -69,10 +68,11 @@ object DatabaseModule { MIGRATION_12_13, MIGRATION_13_14, MIGRATION_14_15, + MIGRATION_15_16, ) // No destructive fallback: the migration chain is complete, and silently dropping the - // accounts/credentials/mail tables would lose stored secrets. A missing migration should - // fail loudly in testing instead. + // mail/message tables would lose cached data. A missing migration should fail loudly in + // testing instead. // Opt-in at-rest encryption of the local cache (off by default). The conversion runs here — // before the database is opened — so it never races an open connection; toggling the setting @@ -92,6 +92,8 @@ object DatabaseModule { // restarts the app; we wipe the cache HERE — at cold start, before Room opens — so the file is // never deleted from under an open connection. Crash-safe order: wipe + reset the seals, and // only THEN clear the flag, so a kill mid-wipe just repeats the idempotent wipe next start. + // Only libremail.db is wiped: accounts/credentials live in AccountDatabase (a separate file), + // so the user stays signed in across the wipe (issue #111). if (runBlocking { keyStore.isClearPending() }) { DatabaseFiles.clear(context) runBlocking { @@ -100,6 +102,12 @@ object DatabaseModule { } } + // One-time move of accounts/credentials/settings/signatures into the non-auth AccountDatabase + // (issue #111). MUST run before builder.build() below: opening the cache applies MIGRATION_15_16, + // which drops the moved tables. It runs AFTER the wipe above so an unrecoverable-key cache is + // gone first (nothing left to move) and we never block waiting on a passphrase we can't get. + runBlocking { accountDataMigrator.migrateIfNeeded() } + val settings = runBlocking { settingsRepository.settings.first() } val appLock = settings.appLock if (settings.encryptCache) { @@ -119,15 +127,6 @@ object DatabaseModule { @Provides fun provideMessageDao(database: LibreMailDatabase): MessageDao = database.messageDao() - @Provides - fun provideAccountDao(database: LibreMailDatabase): AccountDao = database.accountDao() - - @Provides - fun provideAccountSettingsDao(database: LibreMailDatabase): AccountSettingsDao = database.accountSettingsDao() - - @Provides - fun provideCredentialDao(database: LibreMailDatabase): CredentialDao = database.credentialDao() - @Provides fun provideAttachmentDao(database: LibreMailDatabase): AttachmentDao = database.attachmentDao() @@ -140,9 +139,6 @@ object DatabaseModule { @Provides fun provideFolderDao(database: LibreMailDatabase): FolderDao = database.folderDao() - @Provides - fun provideSignatureDao(database: LibreMailDatabase): SignatureDao = database.signatureDao() - @Provides fun provideBackfillProgressDao(database: LibreMailDatabase): BackfillProgressDao = database.backfillProgressDao()