From 94f12820bc8554e4ab4d1e58910d1036d2e2114c Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Thu, 28 Dec 2023 11:05:20 +0100 Subject: [PATCH] Ensure authentication token is always sent when it exists (#193) * Ensure auth token is sent if it is present * Allow using login token for other requests while API token is created * Update version code --- app/build.gradle.kts | 6 ++--- .../mealient/data/auth/impl/AuthRepoImpl.kt | 7 +++-- .../data/auth/impl/AuthStorageImpl.kt | 8 +++++- .../ui/auth/AuthenticationViewModel.kt | 1 + .../data/auth/impl/AuthRepoImplTest.kt | 5 ---- .../data/auth/impl/AuthStorageImplTest.kt | 7 ++++- .../mealient/datasource/SignOutHandler.kt | 6 ----- .../datasource/TokenChangeListener.kt | 6 +++++ .../mealient/datasource/ktor/KtorModule.kt | 4 +-- .../datasource/ktor/SignOutHandlerKtor.kt | 20 -------------- .../ktor/TokenChangeListenerKtor.kt | 26 +++++++++++++++++++ .../metadata/android/en-US/changelogs/34.txt | 2 ++ 12 files changed, 56 insertions(+), 42 deletions(-) delete mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/SignOutHandler.kt create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/TokenChangeListener.kt delete mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/SignOutHandlerKtor.kt create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/TokenChangeListenerKtor.kt create mode 100644 fastlane/metadata/android/en-US/changelogs/34.txt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 11fafbd..19fe075 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -2,7 +2,7 @@ import com.android.build.api.dsl.ManagedVirtualDevice import java.io.FileInputStream -import java.util.* +import java.util.Properties plugins { id("gq.kirmanak.mealient.application") @@ -17,8 +17,8 @@ plugins { android { defaultConfig { applicationId = "gq.kirmanak.mealient" - versionCode = 33 - versionName = "0.4.4" + versionCode = 34 + versionName = "0.4.5" testInstrumentationRunner = "gq.kirmanak.mealient.MealientTestRunner" testInstrumentationRunnerArguments += mapOf("clearPackageData" to "true") resourceConfigurations += listOf("en", "es", "ru", "fr", "nl", "pt", "de") diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt index 5b32790..e489317 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt @@ -4,7 +4,6 @@ import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.datasource.AuthenticationProvider -import gq.kirmanak.mealient.datasource.SignOutHandler import gq.kirmanak.mealient.logging.Logger import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map @@ -14,7 +13,6 @@ class AuthRepoImpl @Inject constructor( private val authStorage: AuthStorage, private val authDataSource: AuthDataSource, private val logger: Logger, - private val signOutHandler: SignOutHandler, private val credentialsLogRedactor: CredentialsLogRedactor, ) : AuthRepo, AuthenticationProvider { @@ -23,12 +21,14 @@ class AuthRepoImpl @Inject constructor( override suspend fun authenticate(email: String, password: String) { logger.v { "authenticate() called" } + credentialsLogRedactor.set(email, password) val token = authDataSource.authenticate(email, password) + credentialsLogRedactor.clear() authStorage.setAuthToken(token) + val apiToken = authDataSource.createApiToken(API_TOKEN_NAME) authStorage.setAuthToken(apiToken) - credentialsLogRedactor.clear() } override suspend fun getAuthToken(): String? = authStorage.getAuthToken() @@ -36,7 +36,6 @@ class AuthRepoImpl @Inject constructor( override suspend fun logout() { logger.v { "logout() called" } authStorage.setAuthToken(null) - signOutHandler.signOut() } companion object { diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt index 93f38c5..8976444 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt @@ -4,6 +4,7 @@ import android.content.SharedPreferences import androidx.annotation.VisibleForTesting import androidx.core.content.edit import gq.kirmanak.mealient.data.auth.AuthStorage +import gq.kirmanak.mealient.datasource.TokenChangeListener import gq.kirmanak.mealient.datastore.DataStoreModule.Companion.ENCRYPTED import gq.kirmanak.mealient.extensions.prefsChangeFlow import gq.kirmanak.mealient.logging.Logger @@ -19,6 +20,7 @@ import javax.inject.Singleton @Singleton class AuthStorageImpl @Inject constructor( @Named(ENCRYPTED) private val sharedPreferences: SharedPreferences, + private val tokenChangeListener: TokenChangeListener, private val logger: Logger, ) : AuthStorage { @@ -28,7 +30,11 @@ class AuthStorageImpl @Inject constructor( .distinctUntilChanged() private val singleThreadDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher() - override suspend fun setAuthToken(authToken: String?) = putString(AUTH_TOKEN_KEY, authToken) + override suspend fun setAuthToken(authToken: String?) { + logger.v { "setAuthToken() called with: authToken = $authToken" } + tokenChangeListener.onTokenChange() + putString(AUTH_TOKEN_KEY, authToken) + } override suspend fun getAuthToken(): String? = getString(AUTH_TOKEN_KEY) diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt b/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt index 203ce39..6bc3f05 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt @@ -26,6 +26,7 @@ class AuthenticationViewModel @Inject constructor( _uiState.value = OperationUiState.Progress() viewModelScope.launch { val result = runCatchingExceptCancel { authRepo.authenticate(email, password) } + logger.d { "Authentication result = $result" } _uiState.value = OperationUiState.fromResult(result) } } diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt index b9f2cc9..7edaa42 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt @@ -4,7 +4,6 @@ import com.google.common.truth.Truth.assertThat import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage -import gq.kirmanak.mealient.datasource.SignOutHandler import gq.kirmanak.mealient.datasource.runCatchingExceptCancel import gq.kirmanak.mealient.test.AuthImplTestData.TEST_API_TOKEN import gq.kirmanak.mealient.test.AuthImplTestData.TEST_PASSWORD @@ -31,9 +30,6 @@ class AuthRepoImplTest : BaseUnitTest() { @MockK(relaxUnitFun = true) lateinit var storage: AuthStorage - @MockK(relaxUnitFun = true) - lateinit var signOutHandler: SignOutHandler - @RelaxedMockK lateinit var credentialsLogRedactor: CredentialsLogRedactor @@ -46,7 +42,6 @@ class AuthRepoImplTest : BaseUnitTest() { authStorage = storage, authDataSource = dataSource, logger = logger, - signOutHandler = signOutHandler, credentialsLogRedactor = credentialsLogRedactor, ) } diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt index 1d5b10b..009a305 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt @@ -8,9 +8,11 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidTest import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl.Companion.AUTH_TOKEN_KEY +import gq.kirmanak.mealient.datasource.TokenChangeListener import gq.kirmanak.mealient.test.AuthImplTestData.TEST_TOKEN import gq.kirmanak.mealient.test.HiltRobolectricTest import io.mockk.MockKAnnotations +import io.mockk.impl.annotations.MockK import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.junit.Before @@ -28,11 +30,14 @@ class AuthStorageImplTest : HiltRobolectricTest() { lateinit var sharedPreferences: SharedPreferences + @MockK + lateinit var tokenChangeListener: TokenChangeListener + @Before fun setUp() { MockKAnnotations.init(this) sharedPreferences = context.getSharedPreferences("test", Context.MODE_PRIVATE) - subject = AuthStorageImpl(sharedPreferences, logger) + subject = AuthStorageImpl(sharedPreferences, tokenChangeListener, logger) } @Test diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/SignOutHandler.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/SignOutHandler.kt deleted file mode 100644 index d0346c3..0000000 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/SignOutHandler.kt +++ /dev/null @@ -1,6 +0,0 @@ -package gq.kirmanak.mealient.datasource - -interface SignOutHandler { - - fun signOut() -} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/TokenChangeListener.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/TokenChangeListener.kt new file mode 100644 index 0000000..e28ca34 --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/TokenChangeListener.kt @@ -0,0 +1,6 @@ +package gq.kirmanak.mealient.datasource + +fun interface TokenChangeListener { + + fun onTokenChange() +} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/KtorModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/KtorModule.kt index a568c29..b9dd54a 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/KtorModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/KtorModule.kt @@ -6,7 +6,7 @@ import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent import dagger.multibindings.IntoSet -import gq.kirmanak.mealient.datasource.SignOutHandler +import gq.kirmanak.mealient.datasource.TokenChangeListener import io.ktor.client.HttpClient import javax.inject.Singleton @@ -37,5 +37,5 @@ internal interface KtorModule { fun bindKtorClientBuilder(impl: KtorClientBuilderImpl) : KtorClientBuilder @Binds - fun bindSignOutHandler(impl: SignOutHandlerKtor) : SignOutHandler + fun bindSignOutHandler(impl: TokenChangeListenerKtor): TokenChangeListener } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/SignOutHandlerKtor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/SignOutHandlerKtor.kt deleted file mode 100644 index d40b414..0000000 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/SignOutHandlerKtor.kt +++ /dev/null @@ -1,20 +0,0 @@ -package gq.kirmanak.mealient.datasource.ktor - -import gq.kirmanak.mealient.datasource.SignOutHandler -import io.ktor.client.HttpClient -import io.ktor.client.plugins.auth.Auth -import io.ktor.client.plugins.auth.providers.BearerAuthProvider -import io.ktor.client.plugins.plugin -import javax.inject.Inject - -internal class SignOutHandlerKtor @Inject constructor( - private val httpClient: HttpClient, -) : SignOutHandler { - - override fun signOut() { - httpClient.plugin(Auth) - .providers - .filterIsInstance() - .forEach { it.clearToken() } - } -} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/TokenChangeListenerKtor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/TokenChangeListenerKtor.kt new file mode 100644 index 0000000..424cc81 --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/ktor/TokenChangeListenerKtor.kt @@ -0,0 +1,26 @@ +package gq.kirmanak.mealient.datasource.ktor + +import gq.kirmanak.mealient.datasource.TokenChangeListener +import gq.kirmanak.mealient.logging.Logger +import io.ktor.client.HttpClient +import io.ktor.client.plugins.auth.Auth +import io.ktor.client.plugins.auth.providers.BearerAuthProvider +import io.ktor.client.plugins.plugin +import javax.inject.Inject + +internal class TokenChangeListenerKtor @Inject constructor( + private val httpClient: HttpClient, + private val logger: Logger, +) : TokenChangeListener { + + override fun onTokenChange() { + logger.v { "onTokenChange() called" } + httpClient.plugin(Auth) + .providers + .filterIsInstance() + .forEach { + logger.d { "onTokenChange(): removing the token" } + it.clearToken() + } + } +} \ No newline at end of file diff --git a/fastlane/metadata/android/en-US/changelogs/34.txt b/fastlane/metadata/android/en-US/changelogs/34.txt new file mode 100644 index 0000000..c65a59e --- /dev/null +++ b/fastlane/metadata/android/en-US/changelogs/34.txt @@ -0,0 +1,2 @@ +Fix authentication issues with some Mealie instances. +Allow sending logs to the developer. \ No newline at end of file