diff --git a/AGENTS.md b/AGENTS.md index 98f7c7f..8fb330f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,6 @@ Kanbn4Droid is an unofficial app to connect to and manipulate data stored in sel - AndroidX Preferences library. - AndroidX SplashScreen library. -- AndroidX Credential Manager library. - Kotlin coroutines (Android dispatcher). ## Current bootstrap status @@ -48,10 +47,12 @@ Kanbn4Droid is an unofficial app to connect to and manipulate data stored in sel - The view has fields to request the user for an instance base URL (http or https, with or without port number) and an API key. - The default base URL is that of the standard Kan.bn instance at https://kan.bn/ - The view tries to check connection with the server at the given base URL using the Kan.bn API healthcheck endpoint. -- The API key is managed using Android's own Credential Manager. +- The API key is stored in app preferences together with the base URL. +- No migration is performed from prior Credential Manager storage, so users must re-enter their API key one time after upgrading. - On success, the view stores the URL and API key pair in preferences and moves over to the boards view. - If there is a URL and API Key pair stored, the view tries to authenticate the user through the API automatically and proceeds to the boards view instantly without showing the login screen if successful. -- Current status: implemented in `MainActivity` with XML views and a temporary boards destination (`BoardsPlaceholderActivity`) while the real boards list view is still pending. +- If startup authentication fails due to invalid credentials then the stored API key is invalidated; transient connectivity/server failures keep the stored key and return to login. +- Current status: implemented in `MainActivity` with XML views and navigation into `BoardsActivity`. **Boards list view** - Displays a list of boards as rounded-square cards with the board's title centered in it. diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 7794d5a..404976a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -46,8 +46,6 @@ dependencies { implementation(libs.androidx.core.ktx) implementation(libs.androidx.core.splashscreen) implementation(libs.androidx.appcompat) - implementation(libs.androidx.credentials) - implementation(libs.androidx.credentials.play.services.auth) implementation(libs.material) implementation(libs.androidx.constraintlayout) implementation(libs.androidx.activity.ktx) diff --git a/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/LoginFlowTest.kt b/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/LoginFlowTest.kt index a8c22c3..2a1e866 100644 --- 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 { key = apiKey @@ -124,6 +182,7 @@ class LoginFlowTest { override suspend fun getApiKey(baseUrl: String): Result = Result.success(key) override suspend fun invalidateApiKey(baseUrl: String): Result { + invalidationCount += 1 key = null return Result.success(Unit) } diff --git a/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/auth/PreferencesApiKeyStoreTest.kt b/app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/auth/PreferencesApiKeyStoreTest.kt new file mode 100644 index 0000000..082003f --- /dev/null +++ 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()) + } +} diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/BoardsActivity.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/BoardsActivity.kt index 2f562b8..b96d73b 100644 --- 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 { diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/MainActivity.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/MainActivity.kt index 1eaf99c..03389f7 100644 --- 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 { diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/ApiKeyStore.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/ApiKeyStore.kt index 5b1e271..07505cd 100644 --- 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 @@ -15,60 +8,33 @@ interface ApiKeyStore { suspend fun invalidateApiKey(baseUrl: String): Result } -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 { 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 { 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 { 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" } } diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/AuthResult.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/auth/AuthResult.kt index 2c3fb50..7a42349 100644 --- 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, + ) } } } diff --git a/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/AuthResultMappingTest.kt b/app/src/test/java/space/hackenslacker/kanbn4droid/app/auth/AuthResultMappingTest.kt index e866223..dd0b433 100644 --- 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) } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a8b91ad..807de98 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -11,7 +11,6 @@ espressoCore = "3.6.1" espressoIntents = "3.6.1" espressoContrib = "3.6.1" coreSplashscreen = "1.0.1" -credentials = "1.3.0" coroutines = "1.10.1" lifecycle = "2.8.7" swiperefreshlayout = "1.1.0" @@ -29,8 +28,6 @@ androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-co androidx-espresso-intents = { group = "androidx.test.espresso", name = "espresso-intents", version.ref = "espressoIntents" } androidx-espresso-contrib = { group = "androidx.test.espresso", name = "espresso-contrib", version.ref = "espressoContrib" } androidx-core-splashscreen = { group = "androidx.core", name = "core-splashscreen", version.ref = "coreSplashscreen" } -androidx-credentials = { group = "androidx.credentials", name = "credentials", version.ref = "credentials" } -androidx-credentials-play-services-auth = { group = "androidx.credentials", name = "credentials-play-services-auth", version.ref = "credentials" } kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "coroutines" } androidx-lifecycle-viewmodel-ktx = { group = "androidx.lifecycle", name = "lifecycle-viewmodel-ktx", version.ref = "lifecycle" } androidx-lifecycle-runtime-ktx = { group = "androidx.lifecycle", name = "lifecycle-runtime-ktx", version.ref = "lifecycle" }