Replace Authenticator with Interceptor

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
 }

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<AuthenticationProvider>,
+) : 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"
+    }
+}

datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt

@@ -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<AuthenticationProvider>,
-) : 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()
-}

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()
 }

datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt

@@ -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()
-
-}

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<Request>()
+        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<Request>()
+        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()
+}