From dd313def962f4efb3df81afc7ab2ed5eed1f10d4 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 16:58:04 +0100 Subject: [PATCH] Replace Authenticator with Interceptor --- .../mealient/datasource/DataSourceModule.kt | 8 +- .../datasource/impl/AuthInterceptor.kt | 43 +++++++ .../datasource/impl/MealieAuthenticator.kt | 49 -------- .../datasource/impl/OkHttpBuilderImpl.kt | 3 - .../datasource/MealieAuthenticatorTest.kt | 97 --------------- .../datasource/impl/AuthInterceptorTest.kt | 111 ++++++++++++++++++ 6 files changed, 159 insertions(+), 152 deletions(-) create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt delete mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt delete mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt create mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt index 2ffa990..73ed120 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt @@ -6,8 +6,9 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet +import gq.kirmanak.mealient.datasource.impl.AuthInterceptor import gq.kirmanak.mealient.datasource.impl.CacheBuilderImpl -import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl import gq.kirmanak.mealient.datasource.impl.OkHttpBuilderImpl import gq.kirmanak.mealient.datasource.impl.RetrofitBuilder @@ -19,7 +20,7 @@ import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1Impl import gq.kirmanak.mealient.datasource.v1.MealieServiceV1 import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.json.Json -import okhttp3.Authenticator +import okhttp3.Interceptor import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import retrofit2.Converter @@ -90,5 +91,6 @@ interface DataSourceModule { @Binds @Singleton - fun bindAuthenticator(mealieAuthenticator: MealieAuthenticator): Authenticator + @IntoSet + fun bindAuthInterceptor(authInterceptor: AuthInterceptor): Interceptor } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt new file mode 100644 index 0000000..89b2acf --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt @@ -0,0 +1,43 @@ +package gq.kirmanak.mealient.datasource.impl + +import androidx.annotation.VisibleForTesting +import gq.kirmanak.mealient.datasource.AuthenticationProvider +import gq.kirmanak.mealient.logging.Logger +import kotlinx.coroutines.runBlocking +import okhttp3.Interceptor +import okhttp3.Response +import javax.inject.Inject +import javax.inject.Provider +import javax.inject.Singleton + +@Singleton +class AuthInterceptor @Inject constructor( + private val logger: Logger, + private val authenticationProviderProvider: Provider, +) : Interceptor { + + private val authenticationProvider: AuthenticationProvider + get() = authenticationProviderProvider.get() + + override fun intercept(chain: Interceptor.Chain): Response { + logger.v { "intercept() was called" } + val header = getAuthHeader() + val request = chain.request().let { + if (header == null) it else it.newBuilder().header(HEADER_NAME, header).build() + } + logger.d { "Sending header $HEADER_NAME=${request.header(HEADER_NAME)}" } + return chain.proceed(request).also { + logger.v { "Response code is ${it.code}" } + if (it.code == 401 && header != null) logout() + } + } + + private fun getAuthHeader() = runBlocking { authenticationProvider.getAuthHeader() } + + private fun logout() = runBlocking { authenticationProvider.logout() } + + companion object { + @VisibleForTesting + const val HEADER_NAME = "Authorization" + } +} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt deleted file mode 100644 index 5fe4cf5..0000000 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt +++ /dev/null @@ -1,49 +0,0 @@ -package gq.kirmanak.mealient.datasource.impl - -import androidx.annotation.VisibleForTesting -import gq.kirmanak.mealient.datasource.AuthenticationProvider -import kotlinx.coroutines.runBlocking -import okhttp3.Authenticator -import okhttp3.Request -import okhttp3.Response -import okhttp3.Route -import javax.inject.Inject -import javax.inject.Provider -import javax.inject.Singleton - -@Singleton -// TODO has to be interceptor, otherwise only public recipes are visible -class MealieAuthenticator @Inject constructor( - private val authenticationProviderProvider: Provider, -) : Authenticator { - - private val authenticationProvider: AuthenticationProvider - get() = authenticationProviderProvider.get() - - override fun authenticate(route: Route?, response: Response): Request? { - val supportsBearer = response.challenges().any { it.scheme == BEARER_SCHEME } - val request = response.request - return when { - request.header(HEADER_NAME) != null -> { - logout() - null - } - supportsBearer -> getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } - else -> null - } - } - - private fun getAuthHeader() = runBlocking { authenticationProvider.getAuthHeader() } - - private fun logout() = runBlocking { authenticationProvider.logout() } - - companion object { - @VisibleForTesting - const val HEADER_NAME = "Authorization" - private const val BEARER_SCHEME = "Bearer" - } -} - -private fun Request.copyWithHeader(name: String, value: String): Request { - return newBuilder().header(name, value).build() -} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt index bb2a9a6..21ca1d1 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt @@ -2,7 +2,6 @@ package gq.kirmanak.mealient.datasource.impl import gq.kirmanak.mealient.datasource.CacheBuilder import gq.kirmanak.mealient.datasource.OkHttpBuilder -import okhttp3.Authenticator import okhttp3.Interceptor import okhttp3.OkHttpClient import javax.inject.Inject @@ -13,12 +12,10 @@ class OkHttpBuilderImpl @Inject constructor( private val cacheBuilder: CacheBuilder, // Use @JvmSuppressWildcards because otherwise dagger can't inject it (https://stackoverflow.com/a/43149382) private val interceptors: Set<@JvmSuppressWildcards Interceptor>, - private val authenticator: Authenticator, ) : OkHttpBuilder { override fun buildOkHttp(): OkHttpClient = OkHttpClient.Builder() .apply { interceptors.forEach(::addNetworkInterceptor) } .cache(cacheBuilder.buildCache()) - .authenticator(authenticator) .build() } \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt deleted file mode 100644 index 708d4c7..0000000 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt +++ /dev/null @@ -1,97 +0,0 @@ -package gq.kirmanak.mealient.datasource - -import com.google.common.truth.Truth.assertThat -import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator -import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator.Companion.HEADER_NAME -import gq.kirmanak.mealient.test.BaseUnitTest -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.impl.annotations.MockK -import okhttp3.Protocol -import okhttp3.Request -import okhttp3.Response -import org.junit.Before -import org.junit.Test - -class MealieAuthenticatorTest : BaseUnitTest() { - - private lateinit var subject: MealieAuthenticator - - @MockK(relaxUnitFun = true) - lateinit var authenticationProvider: AuthenticationProvider - - @Before - override fun setUp() { - super.setUp() - subject = MealieAuthenticator { authenticationProvider } - } - - @Test - fun `when bearer is not supported expect authenticate to return null`() { - val response = buildResponse(challenges = null) - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when no auth header exists expect authenticate to return null`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - val response = buildResponse() - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when no auth header exists expect authenticate to call provider`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - val response = buildResponse() - subject.authenticate(null, response) - coVerify { authenticationProvider.getAuthHeader() } - } - - @Test - fun `when an auth header was set expect authenticate to return null`() { - val response = buildResponse(authHeader = "token") - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when an auth header was set expect authenticate to logout`() { - val response = buildResponse(authHeader = "token") - subject.authenticate(null, response) - coVerify { authenticationProvider.logout() } - } - - @Test - fun `when auth header exists expect authenticate to return request`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - val response = buildResponse() - val actualHeader = subject.authenticate(null, response)?.header(HEADER_NAME) - assertThat(actualHeader).isEqualTo("token") - } - - private fun buildResponse( - url: String = "http://localhost", - code: Int = 401, - message: String = "Unauthorized", - protocol: Protocol = Protocol.HTTP_2, - challenges: String? = "Bearer", - authHeader: String? = null, - ): Response { - val request = buildRequest(authHeader, url) - return Response.Builder().apply { - request(request) - code(code) - message(message) - protocol(protocol) - if (challenges != null) header("WWW-Authenticate", challenges) - }.build() - } - - private fun buildRequest( - authHeader: String? = null, - url: String = "http://localhost", - ): Request = Request.Builder().apply { - url(url) - if (authHeader != null) header(HEADER_NAME, authHeader) - }.build() - -} \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt new file mode 100644 index 0000000..16cb895 --- /dev/null +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt @@ -0,0 +1,111 @@ +package gq.kirmanak.mealient.datasource.impl + +import com.google.common.truth.Truth.assertThat +import gq.kirmanak.mealient.datasource.AuthenticationProvider +import gq.kirmanak.mealient.datasource.impl.AuthInterceptor.Companion.HEADER_NAME +import gq.kirmanak.mealient.test.BaseUnitTest +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import io.mockk.slot +import okhttp3.Interceptor +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import org.junit.Before +import org.junit.Test + +class AuthInterceptorTest : BaseUnitTest() { + + private lateinit var subject: Interceptor + + @MockK(relaxed = true) + lateinit var authenticationProvider: AuthenticationProvider + + @MockK(relaxed = true) + lateinit var chain: Interceptor.Chain + + @Before + override fun setUp() { + super.setUp() + subject = AuthInterceptor(logger) { authenticationProvider } + } + + @Test + fun `when intercept is called expect header to be retrieved`() { + subject.intercept(chain) + coVerify { authenticationProvider.getAuthHeader() } + } + + @Test + fun `when intercept is called and no header expect no header`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + coEvery { chain.request() } returns buildRequest() + val requestSlot = slot() + coEvery { chain.proceed(capture(requestSlot)) } returns buildResponse() + subject.intercept(chain) + assertThat(requestSlot.captured.header(HEADER_NAME)).isNull() + } + + @Test + fun `when intercept is called and no header expect no logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 200) + subject.intercept(chain) + coVerify(inverse = true) { authenticationProvider.logout() } + } + + @Test + fun `when intercept is called with no header and auth fails expect no logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 401) + subject.intercept(chain) + coVerify(inverse = true) { authenticationProvider.logout() } + } + + @Test + fun `when intercept is called and there is a header expect a header`() { + coEvery { authenticationProvider.getAuthHeader() } returns "header" + coEvery { chain.request() } returns buildRequest() + val requestSlot = slot() + coEvery { chain.proceed(capture(requestSlot)) } returns buildResponse() + subject.intercept(chain) + assertThat(requestSlot.captured.header(HEADER_NAME)).isEqualTo("header") + } + + @Test + fun `when intercept is called and there is a header that authenticates expect no logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns "header" + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 200) + subject.intercept(chain) + coVerify(inverse = true) { authenticationProvider.logout() } + } + + @Test + fun `when intercept is called and there was a header but still 401 expect logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns "header" + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 401) + subject.intercept(chain) + coVerify { authenticationProvider.logout() } + } + + private fun buildResponse( + url: String = "http://localhost", + code: Int = 200, + message: String = if (code == 200) "OK" else "Unauthorized", + protocol: Protocol = Protocol.HTTP_2, + ) = Response.Builder().apply { + request(buildRequest(url)) + code(code) + message(message) + protocol(protocol) + }.build() + + private fun buildRequest( + url: String = "http://localhost", + ) = Request.Builder().url(url).build() +} \ No newline at end of file