diff --git a/CHANGELOG.md b/CHANGELOG.md index 57d25796d..69857577f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Chapter and book titles too long to fit in the player now scroll, fading out at the edges, instead of being cut off (turn off with Scroll long titles in Settings → Playback) - [Android] Sign-ins and Hardcover tokens are now encrypted with a key kept in your device's keystore, replacing the deprecated encrypted storage; you stay signed in through the update - Each account keeps its own library sorting and layout +- [Android] Slightly faster startup when signed in ### Deprecated diff --git a/data/account/impl/build.gradle.kts b/data/account/impl/build.gradle.kts index ed208d1d2..25b827d40 100644 --- a/data/account/impl/build.gradle.kts +++ b/data/account/impl/build.gradle.kts @@ -36,6 +36,7 @@ kotlin { implementation(libs.kotlin.test) implementation(libs.assertk) implementation(projects.common.test) + implementation(projects.data.db.test) implementation(libs.bundles.test.impl) implementation(libs.multiplatformsettings.test) implementation(projects.features.settings.test) diff --git a/data/account/impl/src/commonMain/kotlin/app/campfire/account/session/UserSessionRestorer.kt b/data/account/impl/src/commonMain/kotlin/app/campfire/account/session/UserSessionRestorer.kt index 1522c8653..ddcf1e05f 100644 --- a/data/account/impl/src/commonMain/kotlin/app/campfire/account/session/UserSessionRestorer.kt +++ b/data/account/impl/src/commonMain/kotlin/app/campfire/account/session/UserSessionRestorer.kt @@ -4,8 +4,8 @@ package app.campfire.account.session import app.campfire.CampfireDatabase -import app.campfire.account.api.AccountManager import app.campfire.account.server.db.ServerWithUser +import app.campfire.account.storage.TokenStorage import app.campfire.core.coroutines.DispatcherProvider import app.campfire.core.di.AppScope import app.campfire.core.logging.bark @@ -25,7 +25,7 @@ interface UserSessionRestorer { @ContributesBinding(AppScope::class) @Inject class DatabaseUserSessionRestorer( - private val accountManager: AccountManager, + private val tokenStorage: TokenStorage, private val deviceSettings: DeviceSettings, private val db: CampfireDatabase, private val dispatcherProvider: DispatcherProvider, @@ -45,9 +45,11 @@ class DatabaseUserSessionRestorer( return@measureTimedValue UserSession.LoggedOut } - // Validate that this account has valid credentials - val tokens = accountManager.getToken(server.user.id) - if (tokens != null) { + // Only check that a token is stored: reading it means decrypting it, which on Android is a + // round of Android Keystore calls that would hold up the main thread waiting on this restore. + // A stored token that turns out to be unreadable fails its first request, and the refresh + // that follows asks the user to sign in again. + if (tokenStorage.has(server.user.id)) { return@measureTimedValue UserSession.LoggedIn(server.user) } else { return@measureTimedValue UserSession.NeedsAuthentication(server) diff --git a/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/SecureTokenStorage.kt b/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/SecureTokenStorage.kt index 9a595f6a9..bb3f86de4 100644 --- a/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/SecureTokenStorage.kt +++ b/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/SecureTokenStorage.kt @@ -35,6 +35,10 @@ class SecureTokenStorage( return AbsToken(accessToken, refreshToken) } + override suspend fun has(userId: UserId): Boolean { + return settings.hasKey(accessTokenStorageKey(userId)) + } + override suspend fun put(userId: UserId, token: AbsToken) { settings.putString( key = accessTokenStorageKey(userId), diff --git a/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/TokenStorage.kt b/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/TokenStorage.kt index a89e9de7a..92ebd4a04 100644 --- a/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/TokenStorage.kt +++ b/data/account/impl/src/commonMain/kotlin/app/campfire/account/storage/TokenStorage.kt @@ -12,6 +12,12 @@ import app.campfire.core.model.UserId interface TokenStorage { suspend fun get(userId: UserId): AbsToken? + + /** + * Whether a token is stored for [userId]. Unlike [get] this doesn't read the token, so it costs + * no decryption, and a stored token that no longer decrypts still counts. + */ + suspend fun has(userId: UserId): Boolean suspend fun put(userId: UserId, token: AbsToken) suspend fun remove(userId: UserId) } diff --git a/data/account/impl/src/commonTest/kotlin/app/campfire/account/session/DatabaseUserSessionRestorerTest.kt b/data/account/impl/src/commonTest/kotlin/app/campfire/account/session/DatabaseUserSessionRestorerTest.kt new file mode 100644 index 000000000..823cd9180 --- /dev/null +++ b/data/account/impl/src/commonTest/kotlin/app/campfire/account/session/DatabaseUserSessionRestorerTest.kt @@ -0,0 +1,158 @@ +// Copyright 2026, Drew Heavner and the Campfire project contributors +// SPDX-License-Identifier: GPL-3.0-only + +package app.campfire.account.session + +import app.campfire.CampfireDatabase +import app.campfire.account.api.AbsToken +import app.campfire.account.storage.TokenStorage +import app.campfire.common.test.coroutines.asTestDispatcherProvider +import app.campfire.core.model.UserId +import app.campfire.core.session.UserSession +import app.campfire.data.mapping.asDatabaseModel +import app.campfire.db.DatabaseFactory +import app.campfire.db.test.createDriver +import app.campfire.network.models.ServerSettings +import app.campfire.network.models.User as NetworkUser +import app.campfire.network.models.UserPermissions +import app.campfire.settings.test.TestDeviceSettings +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import assertk.assertions.isNull +import assertk.assertions.prop +import kotlin.test.Test +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest + +class DatabaseUserSessionRestorerTest { + + private val deviceSettings = TestDeviceSettings() + private val tokenStorage = UnreadableTokenStorage() + + @Test + fun `signed out without a current user`() = restorerTest { db -> + assertThat(restorer(db).restore()).isEqualTo(UserSession.LoggedOut) + } + + @Test + fun `signed out when the current user's account is gone`() = restorerTest { db -> + deviceSettings.currentUserId = USER + + assertThat(restorer(db).restore()).isEqualTo(UserSession.LoggedOut) + assertThat(deviceSettings.currentUserId).isNull() + } + + @Test + fun `signed in with a stored token without reading it`() = restorerTest { db -> + db.insertAccount(USER) + deviceSettings.currentUserId = USER + tokenStorage.stored += USER + + assertThat(restorer(db).restore()) + .isInstanceOf() + .prop(UserSession.LoggedIn::user) + .prop("id") { it.id } + .isEqualTo(USER) + } + + @Test + fun `needs authentication without a stored token`() = restorerTest { db -> + db.insertAccount(USER) + deviceSettings.currentUserId = USER + + assertThat(restorer(db).restore()) + .isInstanceOf() + .prop(UserSession.NeedsAuthentication::server) + .prop("userId") { it.user.id } + .isEqualTo(USER) + } + + private fun TestScope.restorer(db: CampfireDatabase) = DatabaseUserSessionRestorer( + tokenStorage = tokenStorage, + deviceSettings = deviceSettings, + db = db, + dispatcherProvider = asTestDispatcherProvider(), + ) + + private fun restorerTest(block: suspend TestScope.(CampfireDatabase) -> Unit) = runTest { + val driver = createDriver() + try { + block(DatabaseFactory(driver).build()) + } finally { + driver.close() + } + } + + private suspend fun CampfireDatabase.insertAccount(userId: UserId) { + serversQueries.insert(serverSettings().asDatabaseModel(url = SERVER_URL, userId = userId, name = "Home")) + usersQueries.insert(networkUser(userId).asDatabaseModel(SERVER_URL, "library")) + } + + /** Knows which users have a token, but fails a read: restoring must not decrypt one. */ + private class UnreadableTokenStorage : TokenStorage { + val stored = mutableSetOf() + + override suspend fun get(userId: UserId): AbsToken? = error("Restoring read the token of $userId") + override suspend fun has(userId: UserId): Boolean = userId in stored + override suspend fun put(userId: UserId, token: AbsToken) = error("Restoring wrote a token") + override suspend fun remove(userId: UserId) = error("Restoring removed a token") + } + + private fun networkUser(userId: UserId) = NetworkUser( + id = userId, + username = "listener", + type = "user", + mediaProgress = emptyList(), + seriesHideFromContinueListening = emptyList(), + bookmarks = emptyList(), + isActive = true, + isLocked = false, + lastSeen = 0L, + createdAt = 0L, + permissions = UserPermissions( + download = true, + update = false, + delete = false, + upload = false, + accessAllLibraries = true, + accessAllTags = true, + accessExplicitContent = true, + ), + librariesAccessible = emptyList(), + ) + + private fun serverSettings() = ServerSettings( + id = "server-settings", + scannerFindCovers = false, + scannerCoverProvider = "google", + scannerParseSubtitle = false, + scannerPreferMatchedMetadata = false, + scannerDisableWatcher = false, + storeCoverWithItem = false, + storeMetadataWithItem = false, + metadataFileFormat = "json", + rateLimitLoginRequests = 10, + rateLimitLoginWindow = 600000L, + backupSchedule = "30 1 * * *", + backupsToKeep = 2, + maxBackupSize = 1, + loggerDailyLogsToKeep = 7, + loggerScannerLogsToKeep = 2, + homeBookshelfView = 1, + bookshelfView = 1, + sortingIgnorePrefix = false, + sortingPrefixes = listOf("the"), + chromecastEnabled = false, + dateFormat = "MM/dd/yyyy", + timeFormat = "HH:mm", + language = "en-us", + logLevel = 2, + version = "2.36.0", + ) + + private companion object { + const val USER = "user-1" + const val SERVER_URL = "https://abs.example.com" + } +} diff --git a/data/account/impl/src/commonTest/kotlin/app/campfire/account/storage/SecureTokenStorageTest.kt b/data/account/impl/src/commonTest/kotlin/app/campfire/account/storage/SecureTokenStorageTest.kt new file mode 100644 index 000000000..357741f85 --- /dev/null +++ b/data/account/impl/src/commonTest/kotlin/app/campfire/account/storage/SecureTokenStorageTest.kt @@ -0,0 +1,56 @@ +// Copyright 2026, Drew Heavner and the Campfire project contributors +// SPDX-License-Identifier: GPL-3.0-only + +package app.campfire.account.storage + +import app.campfire.account.api.AbsToken +import app.campfire.common.test.coroutines.asTestDispatcherProvider +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isFalse +import assertk.assertions.isNull +import assertk.assertions.isTrue +import com.russhwolf.settings.MapSettings +import kotlin.test.Test +import kotlinx.coroutines.test.runTest + +class SecureTokenStorageTest { + + @Test + fun `round trips a token`() = runTest { + val storage = SecureTokenStorage(MapSettings(), asTestDispatcherProvider()) + + storage.put(USER, AbsToken("access", "refresh")) + + assertThat(storage.get(USER)).isEqualTo(AbsToken("access", "refresh")) + } + + @Test + fun `a token without a refresh token drops the old one`() = runTest { + val storage = SecureTokenStorage(MapSettings(), asTestDispatcherProvider()) + + storage.put(USER, AbsToken("access", "refresh")) + storage.put(USER, AbsToken("rotated", null)) + + assertThat(storage.get(USER)).isEqualTo(AbsToken("rotated", null)) + } + + @Test + fun `has reports whether a user has a stored token`() = runTest { + val storage = SecureTokenStorage(MapSettings(), asTestDispatcherProvider()) + assertThat(storage.has(USER)).isFalse() + + storage.put(USER, AbsToken("access", "refresh")) + assertThat(storage.has(USER)).isTrue() + assertThat(storage.has(OTHER_USER)).isFalse() + + storage.remove(USER) + assertThat(storage.has(USER)).isFalse() + assertThat(storage.get(USER)).isNull() + } + + private companion object { + const val USER = "user-1" + const val OTHER_USER = "user-2" + } +}