refactor: store API keys in app preferences

2026-03-15 21:42:31 -04:00
parent 30f9ac6b98
commit 5016704627
10 changed files with 214 additions and 85 deletions
--- a/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/LoginFlowTest.kt
+++ b/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/LoginFlowTest.kt
@@ -17,6 +17,7 @@ import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import space.hackenslacker.kanbn4droid.app.auth.AuthFailureReason
 import space.hackenslacker.kanbn4droid.app.auth.ApiKeyStore
 import space.hackenslacker.kanbn4droid.app.auth.AuthResult
 import space.hackenslacker.kanbn4droid.app.auth.KanbnApiClient
@@ -65,16 +66,70 @@ class LoginFlowTest {
    @Test
    fun invalidStoredSessionFallsBackToLoginAndKeepsUrl() {
        val sessionStore = InMemorySessionStore("https://kan.bn/")
+        val keyStore = InMemoryApiKeyStore("bad-key")
        MainActivity.dependencies.sessionStoreFactory = { sessionStore }
-        MainActivity.dependencies.apiKeyStoreFactory = { InMemoryApiKeyStore("bad-key") }
+        MainActivity.dependencies.apiKeyStoreFactory = { keyStore }
        MainActivity.dependencies.apiClientFactory = {
-            FakeApiClient(AuthResult.Failure("Authentication failed. Check your API key."))
+            FakeApiClient(
+                AuthResult.Failure(
+                    message = "Authentication failed. Check your API key.",
+                    reason = AuthFailureReason.Authentication,
+                ),
+            )
        }

        ActivityScenario.launch(MainActivity::class.java)

        onView(withId(R.id.baseUrlInput)).check(matches(withText("https://kan.bn/")))
        onView(withId(R.id.signInButton)).check(matches(isDisplayed()))
+        assertEquals(1, keyStore.invalidationCount)
+        assertEquals(null, keyStore.currentKey)
+    }
+
+    @Test
+    fun connectivityFailureDuringAutoLoginFallsBackWithoutInvalidatingKey() {
+        val sessionStore = InMemorySessionStore("https://kan.bn/")
+        val keyStore = InMemoryApiKeyStore("bad-key")
+        MainActivity.dependencies.sessionStoreFactory = { sessionStore }
+        MainActivity.dependencies.apiKeyStoreFactory = { keyStore }
+        MainActivity.dependencies.apiClientFactory = {
+            FakeApiClient(
+                AuthResult.Failure(
+                    message = "Cannot reach server. Check your connection and URL.",
+                    reason = AuthFailureReason.Connectivity,
+                ),
+            )
+        }
+
+        ActivityScenario.launch(MainActivity::class.java)
+
+        onView(withId(R.id.baseUrlInput)).check(matches(withText("https://kan.bn/")))
+        onView(withId(R.id.signInButton)).check(matches(isDisplayed()))
+        assertEquals(0, keyStore.invalidationCount)
+        assertEquals("bad-key", keyStore.currentKey)
+    }
+
+    @Test
+    fun serverFailureDuringAutoLoginFallsBackWithoutInvalidatingKey() {
+        val sessionStore = InMemorySessionStore("https://kan.bn/")
+        val keyStore = InMemoryApiKeyStore("bad-key")
+        MainActivity.dependencies.sessionStoreFactory = { sessionStore }
+        MainActivity.dependencies.apiKeyStoreFactory = { keyStore }
+        MainActivity.dependencies.apiClientFactory = {
+            FakeApiClient(
+                AuthResult.Failure(
+                    message = "Server error: 500",
+                    reason = AuthFailureReason.Server,
+                ),
+            )
+        }
+
+        ActivityScenario.launch(MainActivity::class.java)
+
+        onView(withId(R.id.baseUrlInput)).check(matches(withText("https://kan.bn/")))
+        onView(withId(R.id.signInButton)).check(matches(isDisplayed()))
+        assertEquals(0, keyStore.invalidationCount)
+        assertEquals("bad-key", keyStore.currentKey)
    }

    @Test
@@ -114,6 +169,9 @@ class LoginFlowTest {
        private var key: String?,
    ) : ApiKeyStore {
        var savedKey: String? = null
+        var invalidationCount: Int = 0
+        val currentKey: String?
+            get() = key

        override suspend fun saveApiKey(baseUrl: String, apiKey: String): Result<Unit> {
            key = apiKey
@@ -124,6 +182,7 @@ class LoginFlowTest {
        override suspend fun getApiKey(baseUrl: String): Result<String?> = Result.success(key)

        override suspend fun invalidateApiKey(baseUrl: String): Result<Unit> {
+            invalidationCount += 1
            key = null
            return Result.success(Unit)
        }
--- a/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/auth/PreferencesApiKeyStoreTest.kt
+++ b/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/auth/PreferencesApiKeyStoreTest.kt
@@ -0,0 +1,61 @@
+package space.hackenslacker.kanbn4droid.app.auth
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class PreferencesApiKeyStoreTest {
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val store = PreferencesApiKeyStore(context)
+
+    @Before
+    fun clearStore() = runBlocking {
+        store.invalidateApiKey("setup").getOrThrow()
+    }
+
+    @Test
+    fun saveThenGetReturnsKey() = runBlocking {
+        val saveResult = store.saveApiKey("https://kan.bn/", "kan_key")
+        val getResult = store.getApiKey("https://kan.bn/")
+
+        assertEquals(true, saveResult.isSuccess)
+        assertEquals("kan_key", getResult.getOrNull())
+    }
+
+    @Test
+    fun invalidateClearsKey() = runBlocking {
+        store.saveApiKey("https://kan.bn/", "kan_key").getOrThrow()
+
+        val invalidateResult = store.invalidateApiKey("https://kan.bn/")
+        val getResult = store.getApiKey("https://kan.bn/")
+
+        assertEquals(true, invalidateResult.isSuccess)
+        assertNull(getResult.getOrNull())
+    }
+
+    @Test
+    fun getReturnsNullWhenAbsent() = runBlocking {
+        val getResult = store.getApiKey("https://kan.bn/")
+
+        assertNull(getResult.getOrNull())
+    }
+
+    @Test
+    fun blankAndMalformedBaseUrlStillWork() = runBlocking {
+        val saveBlankResult = store.saveApiKey("", "kan_key")
+        val getMalformedResult = store.getApiKey("not a url")
+        val invalidateMalformedResult = store.invalidateApiKey("::://")
+        val getAfterInvalidateResult = store.getApiKey("   ")
+
+        assertEquals(true, saveBlankResult.isSuccess)
+        assertEquals("kan_key", getMalformedResult.getOrNull())
+        assertEquals(true, invalidateMalformedResult.isSuccess)
+        assertNull(getAfterInvalidateResult.getOrNull())
+    }
+}
--- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/BoardsActivity.kt
+++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/BoardsActivity.kt
@@ -24,9 +24,9 @@ import com.google.android.material.textfield.TextInputLayout
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
 import space.hackenslacker.kanbn4droid.app.auth.ApiKeyStore
-import space.hackenslacker.kanbn4droid.app.auth.CredentialManagerApiKeyStore
 import space.hackenslacker.kanbn4droid.app.auth.HttpKanbnApiClient
 import space.hackenslacker.kanbn4droid.app.auth.KanbnApiClient
+import space.hackenslacker.kanbn4droid.app.auth.PreferencesApiKeyStore
 import space.hackenslacker.kanbn4droid.app.auth.SessionPreferences
 import space.hackenslacker.kanbn4droid.app.auth.SessionStore
 import space.hackenslacker.kanbn4droid.app.boards.BoardSummary
@@ -257,7 +257,7 @@ class BoardsActivity : AppCompatActivity() {

    protected fun provideApiKeyStore(): ApiKeyStore {
        return MainActivity.dependencies.apiKeyStoreFactory?.invoke(this)
-            ?: CredentialManagerApiKeyStore(this)
+            ?: PreferencesApiKeyStore(this)
    }

    protected fun provideApiClient(): KanbnApiClient {
--- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/MainActivity.kt
+++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/MainActivity.kt
@@ -19,10 +19,11 @@ import kotlinx.coroutines.cancel
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import space.hackenslacker.kanbn4droid.app.auth.ApiKeyStore
+import space.hackenslacker.kanbn4droid.app.auth.AuthFailureReason
 import space.hackenslacker.kanbn4droid.app.auth.AuthResult
-import space.hackenslacker.kanbn4droid.app.auth.CredentialManagerApiKeyStore
 import space.hackenslacker.kanbn4droid.app.auth.HttpKanbnApiClient
 import space.hackenslacker.kanbn4droid.app.auth.KanbnApiClient
+import space.hackenslacker.kanbn4droid.app.auth.PreferencesApiKeyStore
 import space.hackenslacker.kanbn4droid.app.auth.SessionPreferences
 import space.hackenslacker.kanbn4droid.app.auth.SessionStore
 import space.hackenslacker.kanbn4droid.app.auth.UrlNormalizer
@@ -100,15 +101,17 @@ class MainActivity : AppCompatActivity() {
            return false
        }

-        return when (apiClient.healthCheck(storedBaseUrl, storedApiKey)) {
+        return when (val authResult = apiClient.healthCheck(storedBaseUrl, storedApiKey)) {
            AuthResult.Success -> {
                openBoards()
                true
            }

            is AuthResult.Failure -> {
-                withContext(Dispatchers.IO) {
-                    apiKeyStore.invalidateApiKey(storedBaseUrl)
+                if (authResult.reason == AuthFailureReason.Authentication) {
+                    withContext(Dispatchers.IO) {
+                        apiKeyStore.invalidateApiKey(storedBaseUrl)
+                    }
                }
                false
            }
@@ -200,7 +203,7 @@ class MainActivity : AppCompatActivity() {

    protected fun provideApiKeyStore(): ApiKeyStore {
        return dependencies.apiKeyStoreFactory?.invoke(this)
-            ?: CredentialManagerApiKeyStore(this)
+            ?: PreferencesApiKeyStore(this)
    }

    protected fun provideApiClient(): KanbnApiClient {
--- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/ApiKeyStore.kt
+++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/ApiKeyStore.kt
@@ -1,13 +1,6 @@
 package space.hackenslacker.kanbn4droid.app.auth

 import android.content.Context
-import android.content.SharedPreferences
-import androidx.credentials.ClearCredentialStateRequest
-import androidx.credentials.CreatePasswordRequest
-import androidx.credentials.CredentialManager
-import androidx.credentials.GetCredentialRequest
-import androidx.credentials.GetPasswordOption
-import androidx.credentials.PasswordCredential

 interface ApiKeyStore {
    suspend fun saveApiKey(baseUrl: String, apiKey: String): Result<Unit>
@@ -15,60 +8,33 @@ interface ApiKeyStore {
    suspend fun invalidateApiKey(baseUrl: String): Result<Unit>
 }

-class CredentialManagerApiKeyStore(
-    private val context: Context,
-    private val credentialManager: CredentialManager = CredentialManager.create(context),
+class PreferencesApiKeyStore(
+    context: Context,
 ) : ApiKeyStore {
-    private val invalidatedPreferences: SharedPreferences =
-        context.getSharedPreferences(INVALIDATED_PREFS_NAME, Context.MODE_PRIVATE)
+    private val preferences = context.getSharedPreferences(PREFERENCES_NAME, Context.MODE_PRIVATE)

    override suspend fun saveApiKey(baseUrl: String, apiKey: String): Result<Unit> {
        return runCatching {
-            credentialManager.createCredential(
-                context,
-                CreatePasswordRequest(id = CREDENTIAL_ID, password = apiKey),
-            )
-            markInvalidated(baseUrl, false)
+            preferences.edit().putString(API_KEY_PREFERENCE_KEY, apiKey).apply()
            Unit
        }
    }

    override suspend fun getApiKey(baseUrl: String): Result<String?> {
        return runCatching {
-            if (isInvalidated(baseUrl)) {
-                return@runCatching null
-            }
-
-            val response = credentialManager.getCredential(
-                context,
-                GetCredentialRequest(listOf(GetPasswordOption())),
-            )
-
-            val credential = response.credential as? PasswordCredential ?: return@runCatching null
-            if (credential.id == CREDENTIAL_ID) credential.password else null
+            preferences.getString(API_KEY_PREFERENCE_KEY, null)
        }
    }

    override suspend fun invalidateApiKey(baseUrl: String): Result<Unit> {
        return runCatching {
-            markInvalidated(baseUrl, true)
-            credentialManager.clearCredentialState(ClearCredentialStateRequest())
+            preferences.edit().remove(API_KEY_PREFERENCE_KEY).apply()
            Unit
        }
    }

-    private fun isInvalidated(baseUrl: String): Boolean {
-        return invalidatedPreferences.getBoolean(baseUrl.invalidatedKey(), false)
-    }
-
-    private fun markInvalidated(baseUrl: String, invalidated: Boolean) {
-        invalidatedPreferences.edit().putBoolean(baseUrl.invalidatedKey(), invalidated).apply()
-    }
-
-    private fun String.invalidatedKey(): String = "invalidated:$this"
-
    private companion object {
-        private const val INVALIDATED_PREFS_NAME = "kanbn_invalidated_api_keys"
-        private const val CREDENTIAL_ID = "space.hackenslacker.kanbn4droid.api_key"
+        private const val PREFERENCES_NAME = "kanbn_api_key_store"
+        private const val API_KEY_PREFERENCE_KEY = "api_key"
    }
 }
--- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/AuthResult.kt
+++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/AuthResult.kt
@@ -1,18 +1,39 @@
 package space.hackenslacker.kanbn4droid.app.auth

+import java.io.IOException
+import java.net.ConnectException
 import java.net.SocketTimeoutException
 import java.net.UnknownHostException

+enum class AuthFailureReason {
+    Authentication,
+    Connectivity,
+    Server,
+    Unexpected,
+}
+
 sealed interface AuthResult {
    data object Success : AuthResult
-    data class Failure(val message: String) : AuthResult
+    data class Failure(
+        val message: String,
+        val reason: AuthFailureReason,
+    ) : AuthResult
 }

 object AuthErrorMapper {
    fun fromHttpCode(code: Int): AuthResult.Failure {
        return when (code) {
-            401, 403 -> AuthResult.Failure("Authentication failed. Check your API key.")
-            else -> AuthResult.Failure("Server error: $code")
+            401,
+            403,
+            -> AuthResult.Failure(
+                message = "Authentication failed. Check your API key.",
+                reason = AuthFailureReason.Authentication,
+            )
+
+            else -> AuthResult.Failure(
+                message = "Server error: $code",
+                reason = AuthFailureReason.Server,
+            )
        }
    }

@@ -20,9 +41,17 @@ object AuthErrorMapper {
        return when (throwable) {
            is SocketTimeoutException,
            is UnknownHostException,
-            -> AuthResult.Failure("Cannot reach server. Check your connection and URL.")
+            is ConnectException,
+            is IOException,
+            -> AuthResult.Failure(
+                message = "Cannot reach server. Check your connection and URL.",
+                reason = AuthFailureReason.Connectivity,
+            )

-            else -> AuthResult.Failure("Unexpected error. Please try again.")
+            else -> AuthResult.Failure(
+                message = "Unexpected error. Please try again.",
+                reason = AuthFailureReason.Unexpected,
+            )
        }
    }
 }
--- a/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/AuthResultMappingTest.kt
+++ b/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/AuthResultMappingTest.kt
@@ -1,5 +1,7 @@
 package space.hackenslacker.kanbn4droid.app.auth

+import java.io.IOException
+import java.net.ConnectException
 import java.net.SocketTimeoutException
 import java.net.UnknownHostException
 import org.junit.Assert.assertEquals
@@ -9,38 +11,51 @@ class AuthResultMappingTest {

    @Test
    fun mapsUnauthorizedCodesToAuthFailureMessage() {
-        assertEquals(
-            "Authentication failed. Check your API key.",
-            AuthErrorMapper.fromHttpCode(401).message,
-        )
-        assertEquals(
-            "Authentication failed. Check your API key.",
-            AuthErrorMapper.fromHttpCode(403).message,
-        )
+        val unauthorized = AuthErrorMapper.fromHttpCode(401)
+        assertEquals("Authentication failed. Check your API key.", unauthorized.message)
+        assertEquals(AuthFailureReason.Authentication, unauthorized.reason)
+
+        val forbidden = AuthErrorMapper.fromHttpCode(403)
+        assertEquals("Authentication failed. Check your API key.", forbidden.message)
+        assertEquals(AuthFailureReason.Authentication, forbidden.reason)
    }

    @Test
    fun mapsTimeoutAndUnknownHostToConnectivityMessage() {
-        assertEquals(
-            "Cannot reach server. Check your connection and URL.",
-            AuthErrorMapper.fromException(SocketTimeoutException()).message,
-        )
-        assertEquals(
-            "Cannot reach server. Check your connection and URL.",
-            AuthErrorMapper.fromException(UnknownHostException()).message,
-        )
+        val timeoutFailure = AuthErrorMapper.fromException(SocketTimeoutException())
+        assertEquals("Cannot reach server. Check your connection and URL.", timeoutFailure.message)
+        assertEquals(AuthFailureReason.Connectivity, timeoutFailure.reason)
+
+        val unknownHostFailure = AuthErrorMapper.fromException(UnknownHostException())
+        assertEquals("Cannot reach server. Check your connection and URL.", unknownHostFailure.message)
+        assertEquals(AuthFailureReason.Connectivity, unknownHostFailure.reason)
+    }
+
+    @Test
+    fun mapsConnectExceptionToConnectivityMessage() {
+        val connectFailure = AuthErrorMapper.fromException(ConnectException())
+        assertEquals("Cannot reach server. Check your connection and URL.", connectFailure.message)
+        assertEquals(AuthFailureReason.Connectivity, connectFailure.reason)
+    }
+
+    @Test
+    fun mapsIOExceptionToConnectivityMessage() {
+        val ioFailure = AuthErrorMapper.fromException(IOException())
+        assertEquals("Cannot reach server. Check your connection and URL.", ioFailure.message)
+        assertEquals(AuthFailureReason.Connectivity, ioFailure.reason)
    }

    @Test
    fun mapsOtherHttpCodesToServerCodeMessage() {
-        assertEquals("Server error: 500", AuthErrorMapper.fromHttpCode(500).message)
+        val serverFailure = AuthErrorMapper.fromHttpCode(500)
+        assertEquals("Server error: 500", serverFailure.message)
+        assertEquals(AuthFailureReason.Server, serverFailure.reason)
    }

    @Test
    fun mapsUnexpectedExceptionsToGenericMessage() {
-        assertEquals(
-            "Unexpected error. Please try again.",
-            AuthErrorMapper.fromException(IllegalStateException()).message,
-        )
+        val unexpectedFailure = AuthErrorMapper.fromException(IllegalStateException())
+        assertEquals("Unexpected error. Please try again.", unexpectedFailure.message)
+        assertEquals(AuthFailureReason.Unexpected, unexpectedFailure.reason)
    }
 }