From da2963aa86a1398e129e834fa3d36641864bbb65 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sat, 4 Jul 2026 02:22:00 -0500 Subject: [PATCH] fix(auth): drop redundant second Outlook token request on sign-in exchangeToken's authorization-code exchange already requests `openid email offline_access $OUTLOOK_SCOPE`, so the returned access token is an outlook.office.com token usable for IMAP verification and the AuthState already carries the refresh token and expiry. The immediate follow-up refreshForScope(authState, OUTLOOK_SCOPE) was a second round-trip for the same resource that only rotated the just-issued refresh token and added a needless onboarding failure point (a transient network error there failed sign-in after consent + code-exchange had already succeeded). Build OAuthResult directly from the code-exchange tokenResponse (accessToken + authState.jsonSerializeString()), dropping the extra refresh. The durable AuthState is still serialized and persisted for later token refresh; the Graph token remains a distinct resource minted on demand via freshGraphToken. Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/auth/OutlookAuthManager.kt | 12 ++++++++---- .../org/libremail/auth/OutlookAuthManagerTest.kt | 16 +++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt b/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt index 9f74b4f..c3904ec 100644 --- a/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt +++ b/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt @@ -106,12 +106,16 @@ class OutlookAuthManager @Inject constructor(@ApplicationContext private val con val authState = AuthState(response, exception).apply { update(tokenResponse, null) } val email = emailFromIdToken(tokenResponse.idToken) ?: throw IllegalStateException("Could not read the account email from the token") - // Mint an Exchange Online token so the caller can verify the account over IMAP. - val outlook = refreshForScope(authState, OUTLOOK_SCOPE) + // The code exchange above already named the Exchange Online resource ($OUTLOOK_SCOPE), so + // this access token is an outlook.office.com token the caller can verify over IMAP directly. + // Don't re-refresh for the same scope: that second round-trip only rotates the just-issued + // refresh token and adds a needless onboarding failure point. The Graph token is a different + // resource and is minted on demand later (freshGraphToken); the durable AuthState — refresh + // token plus this token's expiry — is serialized here for those later refreshes. return OAuthResult( email = email, - accessToken = outlook.accessToken, - authStateJson = outlook.authStateJson, + accessToken = tokenResponse.accessToken.orEmpty(), + authStateJson = authState.jsonSerializeString(), ) } finally { service.dispose() diff --git a/app/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt b/app/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt index f7c874e..bedcd9e 100644 --- a/app/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt +++ b/app/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt @@ -14,6 +14,7 @@ import io.mockk.mockkConstructor import io.mockk.mockkStatic import io.mockk.runs import io.mockk.unmockkAll +import io.mockk.verify import kotlinx.coroutines.test.runTest import net.openid.appauth.AuthState import net.openid.appauth.AuthorizationException @@ -30,6 +31,7 @@ import org.junit.Test import org.libremail.BuildConfig import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertTrue /** * Drives the Outlook/Microsoft OAuth wrapper in a pure JVM test. The AppAuth types it builds @@ -64,18 +66,23 @@ class OutlookAuthManagerTest { } @Test - fun `exchangeToken mints an Exchange token and reads the account email from the id_token`() = runTest { + fun `exchangeToken builds the result from the code-exchange token without a second refresh`() = runTest { installStatics() stubResponse(authResponseWithCode("auth-code")) stubTokenRequests( - tokenResponse(access = "code-access", idToken = jwt("email" to "me@example.com"), refresh = "rt"), - tokenResponse(access = "outlook-access", idToken = null, refresh = "rt"), + tokenResponse(access = "code-access", idToken = jwt("email" to "me@example.com"), refresh = "durable-rt"), ) val result = authManager().exchangeToken(mockk()) assertEquals("me@example.com", result.email) - assertEquals("outlook-access", result.accessToken) // the Exchange-scoped token, not the code one + // The code exchange already named the Exchange Online resource, so its access token is used + // directly for IMAP verification — not a token from a second, redundant refresh round-trip. + assertEquals("code-access", result.accessToken) + // The durable AuthState — carrying the refresh token for later token refreshes — is persisted. + assertTrue(result.authStateJson.contains("durable-rt")) + // Exactly one token request (the code exchange); the redundant second refresh is gone. + verify(exactly = 1) { anyConstructed().performTokenRequest(any(), any()) } } @Test @@ -84,7 +91,6 @@ class OutlookAuthManagerTest { stubResponse(authResponseWithCode("auth-code")) stubTokenRequests( tokenResponse("code-access", jwt("email" to "", "preferred_username" to "alt@example.com"), "rt"), - tokenResponse("outlook-access", null, "rt"), ) assertEquals("alt@example.com", authManager().exchangeToken(mockk()).email) -- 2.47.3