Skip to content

PM-28053: Ensure any exception thrown during re-auth is an IO exception #6175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
david-livefront merged 1 commit into from
Nov 17, 2025
+41 −3
Merged

PM-28053: Ensure any exception thrown during re-auth is an IO exception #6175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ internal class AuthTokenManager(
}
}

@Throws(IOException::class)
override fun intercept(chain: Interceptor.Chain): Response {
val token = getAccessToken()
?: throw IOException(IllegalStateException(MISSING_TOKEN_MESSAGE))
Expand All @@ -96,9 +97,12 @@ internal class AuthTokenManager(
}

@Synchronized
@Throws(IOException::class)
private fun getAccessToken(): String? = authTokenProvider
.getAuthTokenDataOrNull()
?.let { getAccessToken(authTokenData = it).getOrThrow() }
?.let { authTokenData ->
getAccessToken(authTokenData = authTokenData).getOrElse { throw it.toIoException() }
}

@Synchronized
private fun getAccessToken(authTokenData: AuthTokenData): Result<String> {
Expand All @@ -117,3 +121,12 @@ internal class AuthTokenManager(

private fun Response.shouldSkipAuthentication(): Boolean = this.priorResponse != null
}

/**
* Helper method to ensure the exception is an [IOException].
*/
private fun Throwable.toIoException(): IOException =
when (this) {
is IOException -> this
else -> IOException(this)
}
Comment on lines +128 to +132
Copy link
Contributor

@claude claude bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Missing test coverage and documentation

Details

This new extension function needs:

  1. Direct unit tests - Codecov reports 60% patch coverage. Add tests for:

    • IOException passthrough case (line 130)
    • Non-IOException wrapping case (line 131)

  2. KDoc documentation - Per project guidelines, explain:

    • Why this conversion is needed (Retrofit compatibility)
    • Behavior for each case

Suggested tests:

@Nested
inner class ToIoExceptionExtension {
    @Test
    fun `toIoException returns IOException unchanged`() {
        val original = IOException("test error")
        val result = original.toIoException()
        assertSame(original, result)
    }

    @Test
    fun `toIoException wraps non-IOException in IOException`() {
        val original = IllegalStateException("test error")
        val result = original.toIoException()
        assertTrue(result is IOException)
        assertSame(original, result.cause)
    }
}

Suggested documentation:

/**
 * Converts this [Throwable] to an [IOException] for Retrofit compatibility.
 * 
 * Retrofit's error handling requires [IOException] instances. This extension ensures
 * all exceptions thrown during token refresh are properly wrapped.
 */

29 changes: 27 additions & 2 deletions network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenManagerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.junit.jupiter.api.Assertions.assertThrows
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import retrofit2.HttpException
import java.io.IOException
import java.time.Clock
import java.time.Instant
Expand Down Expand Up @@ -197,7 +198,7 @@ class AuthTokenManagerTest {

@Suppress("MaxLineLength")
@Test
fun `intercept should throw an exception when auth token is expired and refreshAccessTokenSynchronously returns an error`() {
fun `intercept should throw an io exception when auth token is expired and refreshAccessTokenSynchronously returns an error`() {
val errorMessage = "Fail!"
authTokenManager.refreshTokenProvider = object : RefreshTokenProvider {
override fun refreshAccessTokenSynchronously(
Expand All @@ -216,7 +217,31 @@ class AuthTokenManagerTest {
chain = FakeInterceptorChain(request = request),
)
}
assertEquals(errorMessage, throwable?.message)
assertEquals(errorMessage, throwable.message)
}

@Suppress("MaxLineLength")
@Test
fun `intercept should throw a http exception when auth token is expired and refreshAccessTokenSynchronously returns an error`() {
val error = mockk<HttpException>()
authTokenManager.refreshTokenProvider = object : RefreshTokenProvider {
override fun refreshAccessTokenSynchronously(
userId: String,
): Result<String> = error.asFailure()
}
val authTokenData = AuthTokenData(
userId = USER_ID,
accessToken = ACCESS_TOKEN,
expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L,
)
every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData

val throwable = assertThrows(IOException::class.java) {
authTokenManager.intercept(
chain = FakeInterceptorChain(request = request),
)
}
assertEquals(throwable.cause, error)
}

@Suppress("MaxLineLength")
Expand Down
Loading