From 31f4b7b94cba3747321d8182a10e4ed17add48ec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:04:12 +0000 Subject: [PATCH 1/5] Fix stale-token 403s: check JWT expiry and refresh proactively on every call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getToken() validated the iss claim but never checked exp, so an expired token was returned as valid forever. That broke userIsLoggedIn()'s only trigger condition (getToken() == null), making the PATCH /session refresh a no-op once a session existed — exactly the failure NavGraph's per-route call was meant to prevent. waitForToken(), the single choke point both AuthInterceptor (REST) and RealtimeClient (WebSocket) go through to attach auth, now actively calls userIsLoggedIn() whenever it has no valid cached token, instead of passing waiting on a session-established flow that nothing was flipping. This covers every HTTP/WS call without needing to sprinkle userIsLoggedIn() calls across call sites, since apiService and RealtimeClient are the only two network egress paths in the app. --- .../email/data/auth/AuthressLoginClient.kt | 36 +++++++++++-------- .../email/data/remote/api/AuthInterceptor.kt | 17 ++++----- .../email/presentation/navigation/NavGraph.kt | 7 ++-- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt b/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt index 30c64d2..e0a2a21 100644 --- a/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt +++ b/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt @@ -10,7 +10,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.first import kotlinx.coroutines.withTimeoutOrNull import kotlinx.coroutines.withContext import okhttp3.MediaType.Companion.toMediaType @@ -364,12 +363,14 @@ class AuthressLoginClient( /** * The bearer token for API calls, read from the `authorization` cookie and - * checked against the issuer, as the SDK's getToken does. + * checked against the issuer and expiry, as the SDK's getToken does. + * [JwtManager.decode] already shortens `exp` by a 10s clock-skew buffer. */ fun getToken(): String? { val token = cookieJar.authorizationCookie() ?: return null val payload = JwtManager.decode(token) ?: return null if (payload.optString("iss") != origin) return null + if (payload.has("exp") && payload.getLong("exp") * 1000 <= System.currentTimeMillis()) return null return token } @@ -398,15 +399,24 @@ class AuthressLoginClient( } /** - * Waits until a bearer token is available, then returns it. Suspends until - * [authenticate] plus [completeAuthenticationRequest], or [userIsLoggedIn], - * establishes a session. This is the SDK's documented way to obtain the value - * for an Authorization header, and its one legitimate caller is - * [ch.rhosys.email.data.remote.api.AuthInterceptor] — the HTTP call wrapper - * grabbing a token right before an Email API request goes out. It must never - * be called from Authress's own client: a call like `POST /authentication` is - * what establishes the session, so waiting on its own result here would just - * deadlock until the timeout. + * Waits until a bearer token is available, then returns it. When the cached + * token is missing or expired, this actively revalidates via + * [userIsLoggedIn] (PATCH /session) rather than passively waiting for some + * other caller to refresh it — every HTTP/WebSocket call goes through this + * function, so this is the one choke point that makes an expired token get + * refreshed instead of reused. [userIsLoggedIn] itself no-ops (no network + * call) whenever a valid cached token already exists, so a burst of + * concurrent callers only pays for a PATCH /session while none of them has + * one yet. + * + * This is the SDK's documented way to obtain the value for an Authorization + * header, and its legitimate callers are + * [ch.rhosys.email.data.remote.api.AuthInterceptor] (grabbing a token right + * before an Email API request goes out) and + * [ch.rhosys.email.data.realtime.RealtimeClient] (attaching a token to the + * WebSocket handshake). It must never be called from Authress's own client: + * a call like `POST /authentication` is what establishes the session, so + * waiting on its own result here would just deadlock until the timeout. * * Returns null if no token arrives within [timeoutInMillis]; 0 means do not * wait at all, matching the SDK. @@ -416,9 +426,7 @@ class AuthressLoginClient( if (timeoutInMillis == 0L) return null return withTimeoutOrNull(timeoutInMillis) { - // Resolved by completeAuthenticationRequest or a successful session check. - _sessionEstablished.first { it } - getToken() + if (userIsLoggedIn()) getToken() else null } } diff --git a/app/src/main/java/ch/rhosys/email/data/remote/api/AuthInterceptor.kt b/app/src/main/java/ch/rhosys/email/data/remote/api/AuthInterceptor.kt index a614a34..52d822b 100644 --- a/app/src/main/java/ch/rhosys/email/data/remote/api/AuthInterceptor.kt +++ b/app/src/main/java/ch/rhosys/email/data/remote/api/AuthInterceptor.kt @@ -5,14 +5,15 @@ import okhttp3.Interceptor import okhttp3.Response /** - * Attaches the Authress session token to Email API calls — the one place that - * should ever call [ch.rhosys.email.data.auth.AuthressLoginClient.waitForToken]: - * this is the HTTP call wrapper, grabbing the token right before the request - * that needs it. It returns immediately when a token is already cached, and - * otherwise waits briefly for one being established rather than firing a - * request that's certain to be rejected — e.g. a token that expired between - * route changes, while [AppNavHost][ch.rhosys.email.presentation.navigation.AppNavHost]'s - * `userIsLoggedIn()` refresh is still in flight. + * Attaches the Authress session token to Email API calls — the HTTP call + * wrapper that grabs the token right before the request that needs it, via + * [ch.rhosys.email.data.auth.AuthressLoginClient.waitForToken]. (The Email + * API's WebSocket, [ch.rhosys.email.data.realtime.RealtimeClient], is the + * other legitimate caller, attaching a token to the connection handshake the + * same way.) It returns immediately when a token is already cached, and + * otherwise actively revalidates the session — e.g. a token that expired + * between route changes — rather than firing a request that's certain to be + * rejected. * * `runBlocking` is safe here — OkHttp interceptors run on OkHttp's own * dispatcher, never the main thread. It only stays safe because this diff --git a/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt b/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt index 75cb332..8fdd470 100644 --- a/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt +++ b/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt @@ -73,8 +73,11 @@ private fun AppNavHost() { val container = LocalAppContainer.current // The login SDK recommends calling userIsLoggedIn on every route change: it - // is what revalidates the session and refreshes an expired token, via - // PATCH /session. Without it a stale bearer is sent until the app restarts. + // revalidates the session and refreshes an expired token via PATCH /session. + // AuthressLoginClient.waitForToken() now does this too right before any API/ + // WebSocket call, so this isn't the only thing standing between a stale + // bearer and a request — but it keeps the session fresh proactively, ahead + // of whatever the next screen is about to need. val currentRoute = navController.currentBackStackEntryAsState().value?.destination?.route LaunchedEffect(currentRoute) { container.authManager.userIsLoggedIn() From 63b0b4ed0d5f9f3e2ae52eba3ee88248e8d53c5c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:29:48 +0000 Subject: [PATCH 2/5] Remove isSignedIn sync check, always revalidate via userIsLoggedIn A local-only getToken() != null check can't recover from an expired token the way userIsLoggedIn() can (it triggers PATCH /session), so gating auth-required navigation on the sync check risked landing on LOGIN or APP based on stale state. Both RootNavGraph call sites now call the suspend userIsLoggedIn() instead. --- .../ch/rhosys/email/data/auth/AuthressLoginClient.kt | 2 -- .../ch/rhosys/email/presentation/navigation/NavGraph.kt | 9 +++++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt b/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt index e0a2a21..cc7309f 100644 --- a/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt +++ b/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt @@ -374,8 +374,6 @@ class AuthressLoginClient( return token } - val isSignedIn: Boolean get() = getToken() != null - /** The identity token's claims, for showing who is signed in. */ fun getUserIdentity(): JSONObject? { val payload = JwtManager.decode(cookieJar.userCookie()) ?: return null diff --git a/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt b/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt index 8fdd470..458b199 100644 --- a/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt +++ b/app/src/main/java/ch/rhosys/email/presentation/navigation/NavGraph.kt @@ -7,6 +7,7 @@ import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.navigation.NavType import androidx.navigation.compose.NavHost @@ -32,6 +33,7 @@ import ch.rhosys.email.presentation.stats.StatsScreen import ch.rhosys.email.presentation.templates.TemplatesScreen import ch.rhosys.email.presentation.thread.ThreadScreen import kotlinx.coroutines.flow.first +import kotlinx.coroutines.launch private enum class RootGate { LOADING, ONBOARDING, LOGIN, APP } @@ -39,12 +41,13 @@ private enum class RootGate { LOADING, ONBOARDING, LOGIN, APP } fun RootNavGraph() { val container = LocalAppContainer.current var gate by remember { mutableStateOf(RootGate.LOADING) } + val scope = rememberCoroutineScope() LaunchedEffect(Unit) { val onboarded = container.preferencesStore.hasCompletedOnboarding.first() gate = when { !onboarded -> RootGate.ONBOARDING - !container.authManager.isSignedIn -> RootGate.LOGIN + !container.authManager.userIsLoggedIn() -> RootGate.LOGIN else -> RootGate.APP } } @@ -53,7 +56,9 @@ fun RootNavGraph() { RootGate.LOADING -> CircularProgressIndicator() RootGate.ONBOARDING -> DebugLogOverlay(container.appLogger) { OnboardingScreen(onFinished = { - gate = if (container.authManager.isSignedIn) RootGate.APP else RootGate.LOGIN + scope.launch { + gate = if (container.authManager.userIsLoggedIn()) RootGate.APP else RootGate.LOGIN + } }) } RootGate.LOGIN -> DebugLogOverlay(container.appLogger) { From f5df4be654f7bf51e9e7c9f14eb65cd84039a1b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:43:08 +0000 Subject: [PATCH 3/5] Add extensive test coverage for the login/session lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers every success and failure mode of AuthressLoginClient (token expiry/refresh, the full authenticate()/completeAuthenticationRequest() PKCE + deep-link flow including abandoned/mismatched/duplicate redirect edge cases, logout, linkIdentity, profile/devices), JwtManager's decode and anti-abuse hash, AuthInterceptor's header attachment, and RealtimeClient's handshake/reconnect/account-switch behavior — the exact surface behind the 403 bug fixed on this branch. Tests run against a real MockWebServer (redirected via a test-only interceptor, since the client's host is a fixed BuildConfig value) with AuthressCookieJar/AuthStorageManager faked via mockk rather than their real EncryptedSharedPreferences-backed implementations. Robolectric provides the android.util.Base64 (JWT decode) and Context (CustomTabsIntent launch) support the client needs; it isn't required for AuthInterceptor or RealtimeClient, which stay plain JUnit. AuthStorageManager is now injected into AuthressLoginClient the same way cookieJar already is, so tests can substitute a fake instead of touching encrypted storage. --- app/build.gradle.kts | 3 + .../email/data/auth/AuthressLoginClient.kt | 4 +- .../data/auth/AuthressLoginClientFlowTest.kt | 301 ++++++++++++++++++ .../auth/AuthressLoginClientSessionTest.kt | 256 +++++++++++++++ .../auth/AuthressLoginClientTestSupport.kt | 112 +++++++ .../data/auth/AuthressLoginClientTokenTest.kt | 298 +++++++++++++++++ .../rhosys/email/data/auth/JwtManagerTest.kt | 110 +++++++ .../email/data/realtime/RealtimeClientTest.kt | 211 ++++++++++++ .../data/remote/api/AuthInterceptorTest.kt | 83 +++++ .../RedirectToMockServerInterceptor.kt | 27 ++ .../java/ch/rhosys/email/testutil/TestJwt.kt | 32 ++ gradle/libs.versions.toml | 6 + 12 files changed, 1441 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt create mode 100644 app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt create mode 100644 app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt create mode 100644 app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt create mode 100644 app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt create mode 100644 app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt create mode 100644 app/src/test/java/ch/rhosys/email/data/remote/api/AuthInterceptorTest.kt create mode 100644 app/src/test/java/ch/rhosys/email/testutil/RedirectToMockServerInterceptor.kt create mode 100644 app/src/test/java/ch/rhosys/email/testutil/TestJwt.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index a165f4a..b397ec8 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -161,6 +161,9 @@ dependencies { testImplementation(libs.junit5.jupiter) testImplementation(libs.mockk) testImplementation(libs.kotlinx.coroutines.test) + testImplementation(libs.mockwebserver) + testImplementation(libs.robolectric) + testImplementation(libs.androidx.test.core) androidTestImplementation(platform(libs.compose.bom)) androidTestImplementation(libs.androidx.espresso.core) diff --git a/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt b/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt index cc7309f..8d02667 100644 --- a/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt +++ b/app/src/main/java/ch/rhosys/email/data/auth/AuthressLoginClient.kt @@ -52,6 +52,8 @@ class AuthressLoginClient( private val cookieJar: AuthressCookieJar, httpClient: OkHttpClient, private val logger: AppLogger, + /** Injectable like [cookieJar], so tests can substitute a fake instead of touching EncryptedSharedPreferences. */ + private val storage: AuthStorageManager = AuthStorageManager(context), ) { /** The SDK's HttpClient appends /api to the origin; every path below is relative to it. */ private val loginUrl = "https://${BuildConfig.AUTHRESS_CUSTOM_DOMAIN}/api" @@ -60,8 +62,6 @@ class AuthressLoginClient( private val redirectUri = BuildConfig.OAUTH_REDIRECT_URI - private val storage = AuthStorageManager(context) - /** The Authress calls carry the session cookie and must not carry our API bearer. */ private val http = httpClient.newBuilder().cookieJar(cookieJar).build() diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt new file mode 100644 index 0000000..b7fd734 --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt @@ -0,0 +1,301 @@ +package ch.rhosys.email.data.auth + +import android.net.Uri +import androidx.test.core.app.ApplicationProvider +import ch.rhosys.email.testutil.testJwt +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Covers the full sign-in lifecycle: [AuthressLoginClient.authenticate], + * [AuthressLoginClient.completeAuthenticationRequest] and + * [AuthressLoginClient.isRedirect] — every branch of the PKCE + deep-link + * dance, including the abandoned-request and duplicate-redirect edge cases + * the code comments call out as "seen in production". + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class AuthressLoginClientFlowTest { + + private lateinit var server: MockWebServer + private lateinit var cookieBacking: FakeCookieJarBacking + private lateinit var storageBacking: FakeStorageBacking + private lateinit var client: AuthressLoginClient + + @Before + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + server = MockWebServer().apply { start() } + cookieBacking = FakeCookieJarBacking() + storageBacking = FakeStorageBacking() + client = testAuthressLoginClient( + context = ApplicationProvider.getApplicationContext(), + server = server, + cookieJar = mockCookieJar(cookieBacking), + storage = mockStorage(storageBacking), + ) + } + + @After + fun tearDown() { + server.shutdown() + Dispatchers.resetMain() + } + + private fun enqueueAuthenticationResponse(authenticationRequestId: String, authenticationUrl: String = "https://login.rhosys.cloud/continue") { + server.enqueue( + MockResponse().setResponseCode(200).setBody( + JSONObject() + .put("authenticationUrl", authenticationUrl) + .put("authenticationRequestId", authenticationRequestId) + .toString(), + ), + ) + } + + private fun enqueueTokenExchangeResponse(freshToken: String, statusCode: Int = 200) { + server.enqueue( + MockResponse() + .setResponseCode(statusCode) + .apply { if (statusCode in 200..299) addHeader("Set-Cookie", "authorization=$freshToken; Path=/; HttpOnly; Secure") } + .setBody("{}"), + ) + } + + // ── authenticate() ─────────────────────────────────────────────────── + + @Test + fun `authenticate posts PKCE and anti-abuse fields, and ends at AwaitingRedirect`() = runTest { + enqueueAuthenticationResponse("req-1") + + val result = client.authenticate() + + assertTrue(result.isSuccess) + assertEquals(AuthressLoginClient.AuthStatus.AwaitingRedirect, client.authStatus.value) + assertNull(client.authError.value) + + val request = server.takeRequest() + assertEquals("POST", request.method) + assertEquals("/api/authentication", request.path) + val body = JSONObject(request.body.readUtf8()) + assertEquals(ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI, body.getString("redirectUrl")) + assertEquals(ch.rhosys.email.BuildConfig.AUTHRESS_APPLICATION_ID, body.getString("applicationId")) + assertEquals("S256", body.getString("codeChallengeMethod")) + assertTrue(body.getString("codeChallenge").isNotBlank()) + assertTrue(body.getString("antiAbuseHash").startsWith("v2;")) + // Optional fields all omitted by default. + assertFalse(body.has("connectionId")) + + assertEquals("req-1", storageBacking.pending?.authenticationRequestId) + } + + @Test + fun `authenticate includes every optional field when provided`() = runTest { + enqueueAuthenticationResponse("req-1") + + client.authenticate( + AuthressLoginClient.AuthenticationOptions( + connectionId = "conn-1", + tenantLookupIdentifier = "tenant-1", + inviteId = "invite-1", + responseLocation = "somewhere", + flowType = "flow-x", + scopes = listOf("scope-a", "scope-b"), + audiences = listOf("aud-1"), + connectionProperties = mapOf("k" to "v"), + multiAccount = true, + ), + ) + + val body = JSONObject(server.takeRequest().body.readUtf8()) + assertEquals("conn-1", body.getString("connectionId")) + assertEquals("tenant-1", body.getString("tenantLookupIdentifier")) + assertEquals("invite-1", body.getString("inviteId")) + assertEquals("somewhere", body.getString("responseLocation")) + assertEquals("flow-x", body.getString("flowType")) + assertEquals(2, body.getJSONArray("scopes").length()) + assertEquals(1, body.getJSONArray("audiences").length()) + assertEquals("v", body.getJSONObject("connectionProperties").getString("k")) + assertTrue(body.getBoolean("multiAccount")) + } + + @Test + fun `authenticate surfaces a server failure from POST slash authentication`() = runTest { + server.enqueue(MockResponse().setResponseCode(500).setBody("server error")) + + val result = client.authenticate() + + assertTrue(result.isFailure) + assertEquals(AuthressLoginClient.AuthStatus.Idle, client.authStatus.value) + assertNotNull(client.authError.value) + } + + @Test + fun `authenticate re-entering while a previous attempt is in flight abandons it`() = runTest { + enqueueAuthenticationResponse("req-1") + assertTrue(client.authenticate().isSuccess) + assertEquals(AuthressLoginClient.AuthStatus.AwaitingRedirect, client.authStatus.value) + + enqueueAuthenticationResponse("req-2") + assertTrue(client.authenticate().isSuccess) + assertEquals("req-2", storageBacking.pending?.authenticationRequestId) + + // The stale first Custom Tab finally redirects back with req-1's id — it + // should be silently dropped, not surfaced as a mismatch error. + val staleRedirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-1") + val result = client.completeAuthenticationRequest(staleRedirect) + + assertTrue(result.isSuccess) + assertNull(client.authError.value) + // Untouched by the drop: still whatever the second authenticate() left it at. + assertEquals(AuthressLoginClient.AuthStatus.AwaitingRedirect, client.authStatus.value) + } + + // ── completeAuthenticationRequest() ───────────────────────────────── + + @Test + fun `completeAuthenticationRequest exchanges the code and establishes a session`() = runTest { + enqueueAuthenticationResponse("req-1") + client.authenticate() + val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + enqueueTokenExchangeResponse(freshToken) + + val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc123&nonce=req-1") + val result = client.completeAuthenticationRequest(redirect) + + assertTrue(result.isSuccess) + assertEquals(freshToken, client.getToken()) + assertTrue(client.sessionEstablished.value) + assertEquals(AuthressLoginClient.AuthStatus.Idle, client.authStatus.value) + assertEquals(1, cookieBacking.backupCalls) + assertNull(storageBacking.pending) + + server.takeRequest() // the /authentication call + val tokenRequest = server.takeRequest() + assertEquals("POST", tokenRequest.method) + assertEquals("/api/authentication/req-1/tokens", tokenRequest.path) + val body = JSONObject(tokenRequest.body.readUtf8()) + assertEquals("authorization_code", body.getString("grant_type")) + assertEquals("abc123", body.getString("code")) + assertEquals(ch.rhosys.email.BuildConfig.AUTHRESS_APPLICATION_ID, body.getString("client_id")) + } + + @Test + fun `completeAuthenticationRequest fails when there is no pending request`() = runTest { + val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-1") + + val result = client.completeAuthenticationRequest(redirect) + + assertTrue(result.isFailure) + assertEquals("No authentication request in progress (redirect carried authenticationRequestId=req-1)", client.authError.value) + } + + @Test + fun `completeAuthenticationRequest fails on a genuine id mismatch`() = runTest { + storageBacking.pending = AuthStorageManager.PendingAuthentication( + codeVerifier = "verifier", + authenticationRequestId = "req-A", + redirectUrl = ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI, + ) + + val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-B") + val result = client.completeAuthenticationRequest(redirect) + + assertTrue(result.isFailure) + assertEquals("Authentication request mismatch", client.authError.value) + } + + @Test + fun `completeAuthenticationRequest assumes the sole pending request when the redirect carries no id`() = runTest { + storageBacking.pending = AuthStorageManager.PendingAuthentication( + codeVerifier = "verifier", + authenticationRequestId = "req-C", + redirectUrl = ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI, + ) + val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + enqueueTokenExchangeResponse(freshToken) + + val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc") + val result = client.completeAuthenticationRequest(redirect) + + assertTrue(result.isSuccess) + val tokenRequest = server.takeRequest() + assertEquals("/api/authentication/req-C/tokens", tokenRequest.path) + } + + @Test + fun `completeAuthenticationRequest treats a failed exchange as a harmless duplicate when already signed in`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + storageBacking.pending = AuthStorageManager.PendingAuthentication( + codeVerifier = "verifier", + authenticationRequestId = "req-1", + redirectUrl = ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI, + ) + server.enqueue(MockResponse().setResponseCode(409).setBody("duplicate")) + + val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-1") + val result = client.completeAuthenticationRequest(redirect) + + assertTrue(result.isSuccess) + assertTrue(client.sessionEstablished.value) + assertEquals(AuthressLoginClient.AuthStatus.Idle, client.authStatus.value) + assertNull(storageBacking.pending) + } + + @Test + fun `completeAuthenticationRequest surfaces a failed exchange when not already signed in`() = runTest { + storageBacking.pending = AuthStorageManager.PendingAuthentication( + codeVerifier = "verifier", + authenticationRequestId = "req-1", + redirectUrl = ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI, + ) + server.enqueue(MockResponse().setResponseCode(400).setBody("bad code")) + + val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-1") + val result = client.completeAuthenticationRequest(redirect) + + assertTrue(result.isFailure) + assertNotNull(client.authError.value) + // Not cleared on a real failure — nothing in the failure path clears it. + assertNotNull(storageBacking.pending) + } + + // ── isRedirect() ───────────────────────────────────────────────────── + + @Test + fun `isRedirect is false for null`() { + assertFalse(client.isRedirect(null)) + } + + @Test + fun `isRedirect is false for an unrelated uri`() { + assertFalse(client.isRedirect(Uri.parse("https://example.com/callback"))) + } + + @Test + fun `isRedirect is true for the exact redirect uri`() { + assertTrue(client.isRedirect(Uri.parse(ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI))) + } + + @Test + fun `isRedirect is true when the redirect uri carries query params`() { + assertTrue(client.isRedirect(Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-1"))) + } +} diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt new file mode 100644 index 0000000..42a853d --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt @@ -0,0 +1,256 @@ +package ch.rhosys.email.data.auth + +import androidx.test.core.app.ApplicationProvider +import ch.rhosys.email.testutil.testJwt +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.json.JSONArray +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Covers everything past initial sign-in: [AuthressLoginClient.logout], + * [AuthressLoginClient.linkIdentity], [AuthressLoginClient.getUserProfile], + * [AuthressLoginClient.getDevices] and [AuthressLoginClient.deleteDevice]. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class AuthressLoginClientSessionTest { + + private lateinit var server: MockWebServer + private lateinit var cookieBacking: FakeCookieJarBacking + private lateinit var storageBacking: FakeStorageBacking + private lateinit var client: AuthressLoginClient + + @Before + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + server = MockWebServer().apply { start() } + cookieBacking = FakeCookieJarBacking() + storageBacking = FakeStorageBacking() + client = testAuthressLoginClient( + context = ApplicationProvider.getApplicationContext(), + server = server, + cookieJar = mockCookieJar(cookieBacking), + storage = mockStorage(storageBacking), + ) + } + + @After + fun tearDown() { + server.shutdown() + Dispatchers.resetMain() + } + + private fun signIn() { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + } + + // ── logout() ───────────────────────────────────────────────────────── + + @Test + fun `logout deletes the server session and clears local state`() = runTest { + signIn() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + + val result = client.logout() + + assertTrue(result.isSuccess) + val request = server.takeRequest() + assertEquals("DELETE", request.method) + assertEquals("/api/session", request.path) + assertEquals(1, cookieBacking.clearCalls) + assertEquals(1, storageBacking.clearCalls) + assertFalse(client.sessionEstablished.value) + assertNull(client.getToken()) + } + + @Test + fun `logout still clears local state when the server delete call fails`() = runTest { + signIn() + server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) + + val result = client.logout() + + assertTrue(result.isSuccess) + assertEquals(1, cookieBacking.clearCalls) + assertEquals(1, storageBacking.clearCalls) + } + + @Test + fun `logout still clears local state when the server is unreachable`() = runTest { + signIn() + server.shutdown() + + val result = client.logout() + + assertTrue(result.isSuccess) + assertEquals(1, cookieBacking.clearCalls) + assertEquals(1, storageBacking.clearCalls) + } + + // ── linkIdentity() ─────────────────────────────────────────────────── + + @Test + fun `linkIdentity fails when neither connectionId nor tenantLookupIdentifier is given`() = runTest { + signIn() + + val result = client.linkIdentity() + + assertTrue(result.isFailure) + assertEquals(0, server.requestCount) + } + + @Test + fun `linkIdentity fails when not signed in`() = runTest { + val result = client.linkIdentity(connectionId = "conn-1") + + assertTrue(result.isFailure) + assertEquals(0, server.requestCount) + } + + @Test + fun `linkIdentity posts linkIdentity true and stores the new pending request`() = runTest { + signIn() + server.enqueue( + MockResponse().setResponseCode(200).setBody( + JSONObject() + .put("authenticationUrl", "https://login.rhosys.cloud/link") + .put("authenticationRequestId", "link-req-1") + .toString(), + ), + ) + + val result = client.linkIdentity(connectionId = "conn-1") + + assertTrue(result.isSuccess) + assertEquals("link-req-1", result.getOrNull()?.authenticationRequestId) + assertEquals("link-req-1", storageBacking.pending?.authenticationRequestId) + + val request = server.takeRequest() + val body = JSONObject(request.body.readUtf8()) + assertTrue(body.getBoolean("linkIdentity")) + assertEquals("conn-1", body.getString("connectionId")) + } + + // ── getUserProfile() ───────────────────────────────────────────────── + + @Test + fun `getUserProfile fails when not signed in`() = runTest { + val result = client.getUserProfile() + + assertTrue(result.isFailure) + assertEquals(0, server.requestCount) + } + + @Test + fun `getUserProfile returns the profile payload when signed in`() = runTest { + signIn() + server.enqueue(MockResponse().setResponseCode(200).setBody("""{"name":"Alex"}""")) + + val result = client.getUserProfile() + + assertTrue(result.isSuccess) + assertEquals("Alex", result.getOrNull()?.getString("name")) + assertEquals("/api/session/profile", server.takeRequest().path) + } + + // ── getDevices() ───────────────────────────────────────────────────── + + @Test + fun `getDevices returns empty without a network call when not signed in`() = runTest { + val result = client.getDevices() + + assertTrue(result.isSuccess) + assertTrue(result.getOrNull().isNullOrEmpty()) + assertEquals(0, server.requestCount) + } + + @Test + fun `getDevices returns empty on a 401`() = runTest { + signIn() + server.enqueue(MockResponse().setResponseCode(401)) + + val result = client.getDevices() + + assertTrue(result.isSuccess) + assertTrue(result.getOrNull().isNullOrEmpty()) + } + + @Test + fun `getDevices returns empty on a 404`() = runTest { + signIn() + server.enqueue(MockResponse().setResponseCode(404)) + + val result = client.getDevices() + + assertTrue(result.isSuccess) + assertTrue(result.getOrNull().isNullOrEmpty()) + } + + @Test + fun `getDevices surfaces a genuine server error`() = runTest { + signIn() + server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) + + val result = client.getDevices() + + assertTrue(result.isFailure) + } + + @Test + fun `getDevices parses the device list on success`() = runTest { + signIn() + val devices = JSONArray() + .put(JSONObject().put("deviceId", "dev-1").put("name", "Pixel")) + .put(JSONObject().put("deviceId", "dev-2")) + server.enqueue(MockResponse().setResponseCode(200).setBody(JSONObject().put("devices", devices).toString())) + + val result = client.getDevices() + + assertTrue(result.isSuccess) + val list = result.getOrNull()!! + assertEquals(2, list.size) + assertEquals("dev-1", list[0].deviceId) + assertEquals("Pixel", list[0].name) + assertEquals("dev-2", list[1].deviceId) + assertEquals("", list[1].name) + } + + // ── deleteDevice() ─────────────────────────────────────────────────── + + @Test + fun `deleteDevice calls the device-scoped delete endpoint`() = runTest { + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + + val result = client.deleteDevice("dev-1") + + assertTrue(result.isSuccess) + val request = server.takeRequest() + assertEquals("DELETE", request.method) + assertEquals("/api/session/devices/dev-1", request.path) + } + + @Test + fun `deleteDevice surfaces a server failure`() = runTest { + server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) + + val result = client.deleteDevice("dev-1") + + assertTrue(result.isFailure) + } +} diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt new file mode 100644 index 0000000..9bbcd87 --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt @@ -0,0 +1,112 @@ +package ch.rhosys.email.data.auth + +import android.content.Context +import ch.rhosys.email.BuildConfig +import ch.rhosys.email.data.log.AppLogger +import ch.rhosys.email.testutil.RedirectToMockServerInterceptor +import io.mockk.Runs +import io.mockk.captureNullable +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.secondArg +import io.mockk.slot +import okhttp3.Cookie +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockWebServer +import java.util.concurrent.TimeUnit + +/** Origin every test JWT's `iss` claim must match — mirrors `AuthressLoginClient.origin`. */ +const val TEST_ORIGIN = "https://${BuildConfig.AUTHRESS_CUSTOM_DOMAIN}" + +/** + * In-memory stand-in for the two cookies [AuthressCookieJar] persists + * (encrypted, on real devices). Mirrors its actual save/read/clear semantics — + * including "an expiry in the past is a deletion" from `saveFromResponse` — + * closely enough that a PATCH /session response with a `Set-Cookie: + * authorization=...` header really does change what [AuthressLoginClient.getToken] + * sees next, the same way it would in production. + */ +class FakeCookieJarBacking { + val cookies = mutableMapOf() + var backupCalls = 0 + private set + var clearCalls = 0 + private set + + fun recordBackup() { + backupCalls++ + } + + fun recordClear() { + clearCalls++ + cookies.clear() + } +} + +fun mockCookieJar(backing: FakeCookieJarBacking): AuthressCookieJar { + val jar = mockk() + every { jar.authorizationCookie() } answers { backing.cookies["authorization"] } + every { jar.userCookie() } answers { backing.cookies["user"] } + every { jar.saveFromResponse(any(), any()) } answers { + val cookieList = secondArg>() + cookieList.forEach { cookie -> + if (cookie.expiresAt < System.currentTimeMillis()) { + backing.cookies.remove(cookie.name) + } else { + backing.cookies[cookie.name] = cookie.value + } + } + } + every { jar.loadForRequest(any()) } returns emptyList() + every { jar.backupCookies() } answers { backing.recordBackup() } + every { jar.restoreCookies() } just Runs + every { jar.clear() } answers { backing.recordClear() } + return jar +} + +/** In-memory stand-in for [AuthStorageManager]'s encrypted pending-auth-request slot. */ +class FakeStorageBacking { + var pending: AuthStorageManager.PendingAuthentication? = null + var clearCalls = 0 + private set +} + +fun mockStorage(backing: FakeStorageBacking): AuthStorageManager { + val storage = mockk() + val stateSlot = slot() + every { storage.setAuthenticationRequest(captureNullable(stateSlot)) } answers { backing.pending = stateSlot.captured } + every { storage.getAuthenticationRequest() } answers { backing.pending } + every { storage.clear() } answers { backing.clearCalls++; backing.pending = null } + return storage +} + +fun mockLogger(): AppLogger = mockk(relaxed = true) + +/** + * A client wired to [server] via [RedirectToMockServerInterceptor], so requests + * still address the real `AUTHRESS_CUSTOM_DOMAIN` host (keeping `iss` checks + * consistent with production) while physically landing on the local mock + * server. [readTimeoutMillis] lets a couple of tests use a short OkHttp + * read timeout to deterministically simulate a hung/slow server, since a + * synchronous `Call.execute()` inside `withContext(Dispatchers.IO)` is not + * itself interruptible by coroutine cancellation. + */ +fun testAuthressLoginClient( + context: Context, + server: MockWebServer, + cookieJar: AuthressCookieJar, + storage: AuthStorageManager, + logger: AppLogger = mockLogger(), + readTimeoutMillis: Long = 5_000, +): AuthressLoginClient { + val httpClient = OkHttpClient.Builder() + .addInterceptor(RedirectToMockServerInterceptor(server.url("/"))) + .connectTimeout(readTimeoutMillis, TimeUnit.MILLISECONDS) + .readTimeout(readTimeoutMillis, TimeUnit.MILLISECONDS) + .build() + return AuthressLoginClient(context, cookieJar, httpClient, logger, storage) +} + +fun MockWebServer.baseUrl(): HttpUrl = url("/") diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt new file mode 100644 index 0000000..2a66996 --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt @@ -0,0 +1,298 @@ +package ch.rhosys.email.data.auth + +import androidx.test.core.app.ApplicationProvider +import ch.rhosys.email.testutil.testJwt +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.concurrent.TimeUnit + +/** + * Covers [AuthressLoginClient.getToken], [AuthressLoginClient.getUserIdentity], + * [AuthressLoginClient.userIsLoggedIn] and [AuthressLoginClient.waitForToken] — + * the exact surface behind the 403 bug fixed alongside these tests (an expired + * cached token being reused instead of triggering a refresh). Every success + * and failure mode of the local expiry check and the PATCH /session refresh + * path is exercised here. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class AuthressLoginClientTokenTest { + + private lateinit var server: MockWebServer + private lateinit var cookieBacking: FakeCookieJarBacking + private lateinit var storageBacking: FakeStorageBacking + private lateinit var client: AuthressLoginClient + + @Before + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + server = MockWebServer().apply { start() } + cookieBacking = FakeCookieJarBacking() + storageBacking = FakeStorageBacking() + client = testAuthressLoginClient( + context = ApplicationProvider.getApplicationContext(), + server = server, + cookieJar = mockCookieJar(cookieBacking), + storage = mockStorage(storageBacking), + ) + } + + @After + fun tearDown() { + server.shutdown() + Dispatchers.resetMain() + } + + // ── getToken() ──────────────────────────────────────────────────────── + + @Test + fun `getToken returns null when there is no cached cookie`() { + assertNull(client.getToken()) + } + + @Test + fun `getToken returns null for a malformed token`() { + cookieBacking.cookies["authorization"] = "not-a-jwt" + assertNull(client.getToken()) + } + + @Test + fun `getToken returns null when the issuer does not match`() { + cookieBacking.cookies["authorization"] = testJwt("https://someone-else.example", secondsFromNow = 3600) + assertNull(client.getToken()) + } + + @Test + fun `getToken returns null for an already-expired token`() { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -3600) + assertNull(client.getToken()) + } + + @Test + fun `getToken returns null for a token inside the 10s clock-skew buffer`() { + // JwtManager.decode shortens exp by 10s; a token expiring 5s from now is + // therefore already treated as expired. + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = 5) + assertNull(client.getToken()) + } + + @Test + fun `getToken returns the token when valid and not yet expired`() { + val token = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + cookieBacking.cookies["authorization"] = token + assertEquals(token, client.getToken()) + } + + @Test + fun `getToken tolerates a token with no exp claim at all`() { + val token = testJwt(JSONObject().put("iss", TEST_ORIGIN).put("sub", "user-1")) + cookieBacking.cookies["authorization"] = token + assertEquals(token, client.getToken()) + } + + // ── getUserIdentity() ──────────────────────────────────────────────── + + @Test + fun `getUserIdentity returns null when there is no user cookie`() { + assertNull(client.getUserIdentity()) + } + + @Test + fun `getUserIdentity returns null when the issuer does not match`() { + cookieBacking.cookies["user"] = testJwt("https://someone-else.example", secondsFromNow = 3600) + assertNull(client.getUserIdentity()) + } + + @Test + fun `getUserIdentity returns claims for a valid identity token, even if expired`() { + // Unlike getToken, getUserIdentity never checks exp — it's shown to the + // user as "who is signed in", not used to authorize a request. + val token = testJwt( + JSONObject().put("iss", TEST_ORIGIN).put("sub", "user-1").put("exp", 1), + ) + cookieBacking.cookies["user"] = token + val identity = client.getUserIdentity() + assertNotNull(identity) + assertEquals("user-1", identity!!.getString("sub")) + } + + // ── userIsLoggedIn() ───────────────────────────────────────────────── + + @Test + fun `userIsLoggedIn returns true without a network call when a valid token is cached`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + + assertTrue(client.userIsLoggedIn()) + + assertEquals(0, server.requestCount) + } + + @Test + fun `userIsLoggedIn refreshes via PATCH slash session when the cached token is expired`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + server.enqueue( + MockResponse() + .setResponseCode(200) + .addHeader("Set-Cookie", "authorization=$freshToken; Path=/; HttpOnly; Secure") + .setBody("{}"), + ) + + assertTrue(client.userIsLoggedIn()) + + val request = server.takeRequest(1, TimeUnit.SECONDS) + assertNotNull(request) + assertEquals("PATCH", request!!.method) + assertEquals("/api/session", request.path) + assertEquals(freshToken, client.getToken()) + assertTrue(client.sessionEstablished.value) + } + + @Test + fun `userIsLoggedIn refreshes when there is no cookie at all yet`() = runTest { + val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + server.enqueue( + MockResponse() + .setResponseCode(200) + .addHeader("Set-Cookie", "authorization=$freshToken; Path=/; HttpOnly; Secure") + .setBody("{}"), + ) + + assertTrue(client.userIsLoggedIn()) + assertEquals(freshToken, client.getToken()) + } + + @Test + fun `userIsLoggedIn returns false when the server rejects the refresh`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + server.enqueue(MockResponse().setResponseCode(401).setBody("{\"error\":\"invalid session\"}")) + + assertFalse(client.userIsLoggedIn()) + assertNull(client.getToken()) + assertFalse(client.sessionEstablished.value) + } + + @Test + fun `userIsLoggedIn returns false on a 500 from session refresh`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) + + assertFalse(client.userIsLoggedIn()) + } + + @Test + fun `userIsLoggedIn returns false on a network failure talking to the server`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + server.shutdown() + + assertFalse(client.userIsLoggedIn()) + } + + @Test + fun `userIsLoggedIn returns false when the refresh succeeds but the new cookie is still expired`() = runTest { + // Pathological but should not be reported as logged in: the server + // handed back a cookie that is already expired by our clock. + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + val staleReplacement = testJwt(TEST_ORIGIN, secondsFromNow = -1) + server.enqueue( + MockResponse() + .setResponseCode(200) + .addHeader("Set-Cookie", "authorization=$staleReplacement; Path=/; HttpOnly; Secure") + .setBody("{}"), + ) + + assertFalse(client.userIsLoggedIn()) + } + + // ── waitForToken() ─────────────────────────────────────────────────── + + @Test + fun `waitForToken returns the cached token immediately without a network call`() = runTest { + val token = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + cookieBacking.cookies["authorization"] = token + + assertEquals(token, client.waitForToken()) + assertEquals(0, server.requestCount) + } + + @Test + fun `waitForToken with a zero timeout returns null immediately without refreshing`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + + assertNull(client.waitForToken(timeoutInMillis = 0)) + assertEquals(0, server.requestCount) + } + + @Test + fun `waitForToken refreshes an expired token and returns the new one`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) + server.enqueue( + MockResponse() + .setResponseCode(200) + .addHeader("Set-Cookie", "authorization=$freshToken; Path=/; HttpOnly; Secure") + .setBody("{}"), + ) + + assertEquals(freshToken, client.waitForToken()) + } + + @Test + fun `waitForToken returns null promptly when the refresh is rejected, without waiting out the full timeout`() = runTest { + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + server.enqueue(MockResponse().setResponseCode(401)) + + val startedAt = System.currentTimeMillis() + val result = client.waitForToken(timeoutInMillis = 5_000) + val elapsedMs = System.currentTimeMillis() - startedAt + + assertNull(result) + assertTrue("expected a fast failure, took ${elapsedMs}ms", elapsedMs < 2_000) + } + + @Test + fun `waitForToken on a hung server fails within OkHttp's own read timeout, bounding wall time`() = runTest { + // withTimeoutOrNull cannot interrupt a synchronous OkHttp Call.execute() + // mid-flight, so what actually bounds a hung PATCH /session here is the + // client's own read timeout, not waitForToken's timeoutInMillis. This + // pins down that real behavior rather than the parameter's name. + val shortTimeoutClient = testAuthressLoginClient( + context = ApplicationProvider.getApplicationContext(), + server = server, + cookieJar = mockCookieJar(cookieBacking), + storage = mockStorage(storageBacking), + readTimeoutMillis = 300, + ) + cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody("{}") + .setHeadersDelay(5, TimeUnit.SECONDS), + ) + + val startedAt = System.currentTimeMillis() + val result = shortTimeoutClient.waitForToken(timeoutInMillis = 10_000) + val elapsedMs = System.currentTimeMillis() - startedAt + + assertNull(result) + assertTrue("expected the 300ms read timeout to fire, took ${elapsedMs}ms", elapsedMs < 2_000) + } +} diff --git a/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt new file mode 100644 index 0000000..f63bb4d --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt @@ -0,0 +1,110 @@ +package ch.rhosys.email.data.auth + +import ch.rhosys.email.testutil.testJwt +import kotlinx.coroutines.test.runTest +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.security.MessageDigest +import java.util.Base64 + +/** + * [JwtManager] backs both [AuthressLoginClient.getToken]'s expiry check and + * the PKCE/anti-abuse machinery `authenticate()` depends on — its edge cases + * are exercised directly here rather than only indirectly through the client. + */ +@RunWith(RobolectricTestRunner::class) +class JwtManagerTest { + + @Test + fun `decode returns null for a null token`() { + assertNull(JwtManager.decode(null)) + } + + @Test + fun `decode returns null for a blank token`() { + assertNull(JwtManager.decode(" ")) + } + + @Test + fun `decode returns null when there is no payload segment`() { + assertNull(JwtManager.decode("onlyheader")) + } + + @Test + fun `decode returns null for invalid base64`() { + assertNull(JwtManager.decode("header.not-valid-base64!!!.sig")) + } + + @Test + fun `decode returns null when the payload is not JSON`() { + val notJson = Base64.getUrlEncoder().withoutPadding().encodeToString("not json".toByteArray()) + assertNull(JwtManager.decode("header.$notJson.sig")) + } + + @Test + fun `decode leaves claims untouched when there is no exp`() { + val token = testJwt(JSONObject().put("iss", "https://x").put("sub", "u1")) + val payload = JwtManager.decode(token) + assertNotNull(payload) + assertFalse(payload!!.has("exp")) + assertEquals("u1", payload.getString("sub")) + } + + @Test + fun `decode shortens exp by 10 seconds`() { + val originalExp = 1_000_000L + val token = testJwt(JSONObject().put("iss", "https://x").put("exp", originalExp)) + val payload = JwtManager.decode(token) + assertEquals(originalExp - 10, payload!!.getLong("exp")) + } + + @Test + fun `getAuthCodes derives the challenge as sha256 of the verifier`() { + val codes = JwtManager.getAuthCodes() + val expectedChallenge = Base64.getUrlEncoder().withoutPadding().encodeToString( + MessageDigest.getInstance("SHA-256").digest(codes.codeVerifier.toByteArray(Charsets.UTF_8)), + ) + assertEquals(expectedChallenge, codes.codeChallenge) + } + + @Test + fun `getAuthCodes produces a different pair on each call`() { + val a = JwtManager.getAuthCodes() + val b = JwtManager.getAuthCodes() + assertTrue(a.codeVerifier != b.codeVerifier) + } + + @Test + fun `calculateAntiAbuseHash produces the v2 format with a hash starting 00`() = runTest { + val hash = JwtManager.calculateAntiAbuseHash(linkedMapOf("applicationId" to "app-1")) + val parts = hash.split(";") + assertEquals(4, parts.size) + assertEquals("v2", parts[0]) + assertTrue(parts[1].toLong() > 0) // timestamp + assertTrue(parts[2].toInt() > 0) // fineTuner + assertTrue(parts[3].startsWith("00")) + } + + @Test + fun `calculateAntiAbuseHash ignores null, empty-string and false values`() = runTest { + // Sanity check that it doesn't throw and still produces a valid hash + // when most props are absent, mirroring authenticate()'s default options. + val hash = JwtManager.calculateAntiAbuseHash( + linkedMapOf( + "connectionId" to null, + "tenantLookupIdentifier" to null, + "inviteId" to null, + "applicationId" to "app-1", + "audiences" to null, + ), + ) + assertTrue(hash.startsWith("v2;")) + } +} diff --git a/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt b/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt new file mode 100644 index 0000000..8e6809e --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt @@ -0,0 +1,211 @@ +package ch.rhosys.email.data.realtime + +import ch.rhosys.email.data.auth.AuthressLoginClient +import ch.rhosys.email.data.log.AppLogger +import io.mockk.coEvery +import io.mockk.mockk +import okhttp3.OkHttpClient +import okhttp3.Response +import okhttp3.WebSocket +import okhttp3.WebSocketListener +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +/** + * [RealtimeClient] is the WebSocket half of the same choke point + * ([AuthressLoginClient.waitForToken]) whose token-expiry bug produced the + * 403 storm this branch fixes — these tests pin down that it actually + * attaches the token/account to the handshake, and that it recovers from a + * rejected handshake (the exact "Expected HTTP 101 ... 403 Forbidden" + * symptom from production) instead of getting stuck. + * + * Real time is used throughout rather than a virtual-time test dispatcher: + * [RealtimeClient] owns its own `CoroutineScope(SupervisorJob() + Dispatchers.IO)` + * rather than accepting an injected one, so its `delay()` calls (ping + * interval, reconnect backoff) are not reachable from a test's virtual clock. + */ +class RealtimeClientTest { + + private lateinit var server: MockWebServer + private lateinit var httpClient: OkHttpClient + + @Before + fun setUp() { + server = MockWebServer().apply { start() } + httpClient = OkHttpClient.Builder().readTimeout(0, TimeUnit.SECONDS).build() + } + + @After + fun tearDown() { + server.shutdown() + } + + private fun wsBaseUrl(): String = server.url("/").toString().trimEnd('/') + + private fun mockAuthManager(token: String?): AuthressLoginClient { + val authManager = mockk() + coEvery { authManager.waitForToken() } returns token + return authManager + } + + /** Records the server-side handshake and lets the test push messages down after it opens. */ + private class RecordingServerListener : WebSocketListener() { + val openLatch = CountDownLatch(1) + val socket = AtomicReference() + + override fun onOpen(webSocket: WebSocket, response: Response) { + socket.set(webSocket) + openLatch.countDown() + } + } + + private fun buildClient( + authManager: AuthressLoginClient, + onThreadUpdated: suspend (accountId: String, threadId: String) -> Unit = { _, _ -> }, + ) = RealtimeClient( + wsBaseUrl = wsBaseUrl(), + httpClient = httpClient, + authManager = authManager, + logger = mockk(relaxed = true), + onThreadUpdated = onThreadUpdated, + ) + + @Test + fun `connect attaches the token and accountId as handshake query params`() { + val serverListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(serverListener)) + val client = buildClient(mockAuthManager("tok-1")) + + client.start("acc-1") + assertTrue(serverListener.openLatch.await(5, TimeUnit.SECONDS)) + + val request = server.takeRequest() + assertEquals("token=tok-1&accountId=acc-1", request.requestUrl!!.query) + + client.stop() + } + + @Test + fun `an empty token from a failed waitForToken still connects rather than never trying`() { + val serverListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(serverListener)) + val client = buildClient(mockAuthManager(null)) + + client.start("acc-1") + assertTrue(serverListener.openLatch.await(5, TimeUnit.SECONDS)) + + val request = server.takeRequest() + assertEquals("token=&accountId=acc-1", request.requestUrl!!.query) + + client.stop() + } + + @Test + fun `a rejected handshake (403) is retried and eventually succeeds`() { + // First attempt: the server rejects the WebSocket upgrade outright — + // OkHttp surfaces this as onFailure with "Expected HTTP 101 ... 403", + // the exact symptom from production. Second attempt succeeds. + server.enqueue(MockResponse().setResponseCode(403)) + val serverListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(serverListener)) + val client = buildClient(mockAuthManager("tok-1")) + + client.start("acc-1") + // INITIAL_RECONNECT_DELAY_MS is a hardcoded 1s, not injectable — allow + // enough real time for the backoff to fire once. + assertTrue(serverListener.openLatch.await(5, TimeUnit.SECONDS)) + + assertEquals(2, server.requestCount) + client.stop() + } + + @Test + fun `switching accounts closes the old socket and reconnects under the new one`() { + val firstServerListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(firstServerListener)) + val secondServerListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(secondServerListener)) + val client = buildClient(mockAuthManager("tok-1")) + + client.start("acc-1") + assertTrue(firstServerListener.openLatch.await(5, TimeUnit.SECONDS)) + server.takeRequest() // acc-1's handshake + + client.start("acc-2") + assertTrue(secondServerListener.openLatch.await(5, TimeUnit.SECONDS)) + + val secondRequest = server.takeRequest() + assertEquals("token=tok-1&accountId=acc-2", secondRequest.requestUrl!!.query) + + client.stop() + } + + @Test + fun `starting again for the same already-connected account is a no-op`() { + val serverListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(serverListener)) + val client = buildClient(mockAuthManager("tok-1")) + + client.start("acc-1") + assertTrue(serverListener.openLatch.await(5, TimeUnit.SECONDS)) + + client.start("acc-1") + + // No second handshake was ever enqueued/consumed; a second connect + // attempt here would deadlock waiting on a response that isn't there, + // so reaching this line at all proves the no-op held. + assertEquals(1, server.requestCount) + client.stop() + } + + @Test + fun `a thread updated message invokes the callback with the current account and thread id`() { + val serverListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(serverListener)) + val received = CountDownLatch(1) + val seenAccountId = AtomicReference() + val seenThreadId = AtomicReference() + val client = buildClient(mockAuthManager("tok-1")) { accountId, threadId -> + seenAccountId.set(accountId) + seenThreadId.set(threadId) + received.countDown() + } + + client.start("acc-1") + assertTrue(serverListener.openLatch.await(5, TimeUnit.SECONDS)) + serverListener.socket.get().send("""{"type":"thread:updated","threadId":"thr-1"}""") + + assertTrue(received.await(5, TimeUnit.SECONDS)) + assertEquals("acc-1", seenAccountId.get()) + assertEquals("thr-1", seenThreadId.get()) + + client.stop() + } + + @Test + fun `stop closes the socket and start after stop reconnects`() { + val firstServerListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(firstServerListener)) + val client = buildClient(mockAuthManager("tok-1")) + + client.start("acc-1") + assertTrue(firstServerListener.openLatch.await(5, TimeUnit.SECONDS)) + client.stop() + + val secondServerListener = RecordingServerListener() + server.enqueue(MockResponse().withWebSocketUpgrade(secondServerListener)) + client.start("acc-1") + + assertTrue(secondServerListener.openLatch.await(5, TimeUnit.SECONDS)) + assertEquals(2, server.requestCount) + client.stop() + } +} diff --git a/app/src/test/java/ch/rhosys/email/data/remote/api/AuthInterceptorTest.kt b/app/src/test/java/ch/rhosys/email/data/remote/api/AuthInterceptorTest.kt new file mode 100644 index 0000000..62badc6 --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/data/remote/api/AuthInterceptorTest.kt @@ -0,0 +1,83 @@ +package ch.rhosys.email.data.remote.api + +import kotlinx.coroutines.delay +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows +import org.junit.Before +import org.junit.Test + +/** + * [AuthInterceptor] is the one place (besides [ch.rhosys.email.data.realtime.RealtimeClient]) + * that attaches auth to an outgoing Email API call — every case here is + * exercised through a real [OkHttpClient] call against a [MockWebServer], not + * just by invoking `intercept()` directly, so it also proves the header + * really reaches the wire. + */ +class AuthInterceptorTest { + + private lateinit var server: MockWebServer + + @Before + fun setUp() { + server = MockWebServer().apply { start() } + } + + @After + fun tearDown() { + server.shutdown() + } + + private fun clientWith(tokenProvider: suspend () -> String?): OkHttpClient = + OkHttpClient.Builder().addInterceptor(AuthInterceptor(tokenProvider)).build() + + @Test + fun `attaches a Bearer header when a token is available`() { + server.enqueue(MockResponse().setResponseCode(200)) + val client = clientWith { "token-123" } + + client.newCall(Request.Builder().url(server.url("/threads")).build()).execute().close() + + val request = server.takeRequest() + assertEquals("Bearer token-123", request.getHeader("Authorization")) + } + + @Test + fun `sends no Authorization header when the token provider returns null`() { + server.enqueue(MockResponse().setResponseCode(200)) + val client = clientWith { null } + + client.newCall(Request.Builder().url(server.url("/threads")).build()).execute().close() + + val request = server.takeRequest() + assertNull(request.getHeader("Authorization")) + } + + @Test + fun `waits for a suspending token provider before sending the request`() { + server.enqueue(MockResponse().setResponseCode(200)) + val client = clientWith { + delay(50) + "delayed-token" + } + + client.newCall(Request.Builder().url(server.url("/threads")).build()).execute().close() + + assertEquals("Bearer delayed-token", server.takeRequest().getHeader("Authorization")) + } + + @Test + fun `propagates a token provider failure as a failed call, without ever reaching the server`() { + val client = clientWith { throw IllegalStateException("token refresh exploded") } + + assertThrows(IllegalStateException::class.java) { + client.newCall(Request.Builder().url(server.url("/threads")).build()).execute() + } + assertEquals(0, server.requestCount) + } +} diff --git a/app/src/test/java/ch/rhosys/email/testutil/RedirectToMockServerInterceptor.kt b/app/src/test/java/ch/rhosys/email/testutil/RedirectToMockServerInterceptor.kt new file mode 100644 index 0000000..0d5d623 --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/testutil/RedirectToMockServerInterceptor.kt @@ -0,0 +1,27 @@ +package ch.rhosys.email.testutil + +import okhttp3.HttpUrl +import okhttp3.Interceptor +import okhttp3.Response + +/** + * [AuthressLoginClient][ch.rhosys.email.data.auth.AuthressLoginClient] builds + * its request URLs from a fixed `BuildConfig.AUTHRESS_CUSTOM_DOMAIN` host, not + * an injectable base URL — matching production, where that host never + * changes at runtime. Rather than adding test-only seams to production code, + * this interceptor keeps the client pointed at the real host end to end (so + * `iss` checks against `origin` still line up) and only swaps the physical + * scheme/host/port onto a local [okhttp3.mockwebserver.MockWebServer] right + * before the request leaves the process. + */ +class RedirectToMockServerInterceptor(private val target: HttpUrl) : Interceptor { + override fun intercept(chain: Interceptor.Chain): Response { + val original = chain.request() + val redirected = original.url.newBuilder() + .scheme(target.scheme) + .host(target.host) + .port(target.port) + .build() + return chain.proceed(original.newBuilder().url(redirected).build()) + } +} diff --git a/app/src/test/java/ch/rhosys/email/testutil/TestJwt.kt b/app/src/test/java/ch/rhosys/email/testutil/TestJwt.kt new file mode 100644 index 0000000..bf2dab1 --- /dev/null +++ b/app/src/test/java/ch/rhosys/email/testutil/TestJwt.kt @@ -0,0 +1,32 @@ +package ch.rhosys.email.testutil + +import org.json.JSONObject +import java.util.Base64 + +/** + * Builds an unsigned JWT string (header.payload.signature) good enough for + * [ch.rhosys.email.data.auth.JwtManager.decode] to parse — it never verifies + * the signature, only base64url-decodes the payload segment, matching the SDK + * it ports. Encoding here uses [java.util.Base64]'s URL-safe, no-padding + * encoder, which produces the same output `android.util.Base64` does under + * Robolectric's shadow with `URL_SAFE or NO_PADDING or NO_WRAP`. + */ +fun testJwt(claims: JSONObject): String { + val encoder = Base64.getUrlEncoder().withoutPadding() + val header = encoder.encodeToString("""{"alg":"none","typ":"JWT"}""".toByteArray(Charsets.UTF_8)) + val payload = encoder.encodeToString(claims.toString().toByteArray(Charsets.UTF_8)) + return "$header.$payload.signature" +} + +/** + * A token whose `exp` is [secondsFromNow] in the future (or past, if negative), + * issued by [issuer], for the given [subject]. `iss` and `exp` are the only + * claims [ch.rhosys.email.data.auth.AuthressLoginClient.getToken] inspects. + */ +fun testJwt(issuer: String, secondsFromNow: Long, subject: String = "user-1"): String = + testJwt( + JSONObject() + .put("iss", issuer) + .put("sub", subject) + .put("exp", (System.currentTimeMillis() / 1000) + secondsFromNow), + ) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 126c4b7..037943c 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -24,6 +24,9 @@ posthog = "3.9.1" junit = "4.13.2" junit5 = "5.10.2" mockk = "1.13.11" +mockwebserver = "4.12.0" +robolectric = "4.14.1" +androidx-test-core = "1.6.1" espresso = "3.6.1" wear-compose = "1.4.0" play-services-wearable = "18.2.0" @@ -82,6 +85,9 @@ posthog-android = { module = "com.posthog:posthog-android", junit = { module = "junit:junit", version.ref = "junit" } junit5-jupiter = { module = "org.junit.jupiter:junit-jupiter", version.ref = "junit5" } mockk = { module = "io.mockk:mockk", version.ref = "mockk" } +mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "mockwebserver" } +robolectric = { module = "org.robolectric:robolectric", version.ref = "robolectric" } +androidx-test-core = { module = "androidx.test:core", version.ref = "androidx-test-core" } androidx-espresso-core = { module = "androidx.test.espresso:espresso-core", version.ref = "espresso" } compose-ui-test-junit4 = { module = "androidx.compose.ui:ui-test-junit4" } compose-ui-test-manifest = { module = "androidx.compose.ui:ui-test-manifest" } From 244485975d594f1e858a6ba54fb6625315d8ef22 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:47:46 +0000 Subject: [PATCH 4/5] Fix test compile error: captureNullable/secondArg aren't top-level mockk imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are members of MockKAnswerScope, resolved via the answers{} block's implicit receiver — importing them as free functions doesn't compile. Switched the pending-auth-request stub to any()+firstArg() (any() already matches null), and dropped the bogus secondArg import since the bare call inside answers{} was already correct. --- .../email/data/auth/AuthressLoginClientTestSupport.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt index 9bbcd87..ef2da82 100644 --- a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTestSupport.kt @@ -5,12 +5,9 @@ import ch.rhosys.email.BuildConfig import ch.rhosys.email.data.log.AppLogger import ch.rhosys.email.testutil.RedirectToMockServerInterceptor import io.mockk.Runs -import io.mockk.captureNullable import io.mockk.every import io.mockk.just import io.mockk.mockk -import io.mockk.secondArg -import io.mockk.slot import okhttp3.Cookie import okhttp3.HttpUrl import okhttp3.OkHttpClient @@ -75,8 +72,9 @@ class FakeStorageBacking { fun mockStorage(backing: FakeStorageBacking): AuthStorageManager { val storage = mockk() - val stateSlot = slot() - every { storage.setAuthenticationRequest(captureNullable(stateSlot)) } answers { backing.pending = stateSlot.captured } + // any() matches a null argument too, so this covers both the "clear pending" + // and "set pending" calls without needing a capture matcher for a nullable type. + every { storage.setAuthenticationRequest(any()) } answers { backing.pending = firstArg() } every { storage.getAuthenticationRequest() } answers { backing.pending } every { storage.clear() } answers { backing.clearCalls++; backing.pending = null } return storage From ce32a14f91b3c5c05328bab93b76cd1f0809a52a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:53:43 +0000 Subject: [PATCH 5/5] Fix two test-runtime failures from the first CI run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Switch runTest to runBlocking across the AuthressLoginClient/JwtManager test suites: runTest's virtual-time scheduler auto-advances past the real Dispatchers.IO network call inside AuthressLoginClient.execute(), which made withTimeoutOrNull in waitForToken() fire its timeout before the real MockWebServer response arrived (seen failing: "waitForToken refreshes an expired token and returns the new one"). These tests do real I/O and never use virtual-time control, so runBlocking is the correct tool. - RealtimeClientTest: MockWebServer.shutdown() can throw IOException in tearDown when a WebSocket's close handshake (from client.stop()) hasn't finished yet. Harmless — wrap in runCatching. --- .../data/auth/AuthressLoginClientFlowTest.kt | 22 ++++++------- .../auth/AuthressLoginClientSessionTest.kt | 32 +++++++++---------- .../data/auth/AuthressLoginClientTokenTest.kt | 26 +++++++-------- .../rhosys/email/data/auth/JwtManagerTest.kt | 6 ++-- .../email/data/realtime/RealtimeClientTest.kt | 7 +++- 5 files changed, 49 insertions(+), 44 deletions(-) diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt index b7fd734..5bf48a9 100644 --- a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientFlowTest.kt @@ -7,7 +7,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain -import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.setMain import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -82,7 +82,7 @@ class AuthressLoginClientFlowTest { // ── authenticate() ─────────────────────────────────────────────────── @Test - fun `authenticate posts PKCE and anti-abuse fields, and ends at AwaitingRedirect`() = runTest { + fun `authenticate posts PKCE and anti-abuse fields, and ends at AwaitingRedirect`() = runBlocking { enqueueAuthenticationResponse("req-1") val result = client.authenticate() @@ -107,7 +107,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `authenticate includes every optional field when provided`() = runTest { + fun `authenticate includes every optional field when provided`() = runBlocking { enqueueAuthenticationResponse("req-1") client.authenticate( @@ -137,7 +137,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `authenticate surfaces a server failure from POST slash authentication`() = runTest { + fun `authenticate surfaces a server failure from POST slash authentication`() = runBlocking { server.enqueue(MockResponse().setResponseCode(500).setBody("server error")) val result = client.authenticate() @@ -148,7 +148,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `authenticate re-entering while a previous attempt is in flight abandons it`() = runTest { + fun `authenticate re-entering while a previous attempt is in flight abandons it`() = runBlocking { enqueueAuthenticationResponse("req-1") assertTrue(client.authenticate().isSuccess) assertEquals(AuthressLoginClient.AuthStatus.AwaitingRedirect, client.authStatus.value) @@ -171,7 +171,7 @@ class AuthressLoginClientFlowTest { // ── completeAuthenticationRequest() ───────────────────────────────── @Test - fun `completeAuthenticationRequest exchanges the code and establishes a session`() = runTest { + fun `completeAuthenticationRequest exchanges the code and establishes a session`() = runBlocking { enqueueAuthenticationResponse("req-1") client.authenticate() val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) @@ -198,7 +198,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `completeAuthenticationRequest fails when there is no pending request`() = runTest { + fun `completeAuthenticationRequest fails when there is no pending request`() = runBlocking { val redirect = Uri.parse("${ch.rhosys.email.BuildConfig.OAUTH_REDIRECT_URI}?code=abc&nonce=req-1") val result = client.completeAuthenticationRequest(redirect) @@ -208,7 +208,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `completeAuthenticationRequest fails on a genuine id mismatch`() = runTest { + fun `completeAuthenticationRequest fails on a genuine id mismatch`() = runBlocking { storageBacking.pending = AuthStorageManager.PendingAuthentication( codeVerifier = "verifier", authenticationRequestId = "req-A", @@ -223,7 +223,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `completeAuthenticationRequest assumes the sole pending request when the redirect carries no id`() = runTest { + fun `completeAuthenticationRequest assumes the sole pending request when the redirect carries no id`() = runBlocking { storageBacking.pending = AuthStorageManager.PendingAuthentication( codeVerifier = "verifier", authenticationRequestId = "req-C", @@ -241,7 +241,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `completeAuthenticationRequest treats a failed exchange as a harmless duplicate when already signed in`() = runTest { + fun `completeAuthenticationRequest treats a failed exchange as a harmless duplicate when already signed in`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = 3600) storageBacking.pending = AuthStorageManager.PendingAuthentication( codeVerifier = "verifier", @@ -260,7 +260,7 @@ class AuthressLoginClientFlowTest { } @Test - fun `completeAuthenticationRequest surfaces a failed exchange when not already signed in`() = runTest { + fun `completeAuthenticationRequest surfaces a failed exchange when not already signed in`() = runBlocking { storageBacking.pending = AuthStorageManager.PendingAuthentication( codeVerifier = "verifier", authenticationRequestId = "req-1", diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt index 42a853d..af7921a 100644 --- a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientSessionTest.kt @@ -6,7 +6,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain -import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.setMain import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -63,7 +63,7 @@ class AuthressLoginClientSessionTest { // ── logout() ───────────────────────────────────────────────────────── @Test - fun `logout deletes the server session and clears local state`() = runTest { + fun `logout deletes the server session and clears local state`() = runBlocking { signIn() server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) @@ -80,7 +80,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `logout still clears local state when the server delete call fails`() = runTest { + fun `logout still clears local state when the server delete call fails`() = runBlocking { signIn() server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) @@ -92,7 +92,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `logout still clears local state when the server is unreachable`() = runTest { + fun `logout still clears local state when the server is unreachable`() = runBlocking { signIn() server.shutdown() @@ -106,7 +106,7 @@ class AuthressLoginClientSessionTest { // ── linkIdentity() ─────────────────────────────────────────────────── @Test - fun `linkIdentity fails when neither connectionId nor tenantLookupIdentifier is given`() = runTest { + fun `linkIdentity fails when neither connectionId nor tenantLookupIdentifier is given`() = runBlocking { signIn() val result = client.linkIdentity() @@ -116,7 +116,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `linkIdentity fails when not signed in`() = runTest { + fun `linkIdentity fails when not signed in`() = runBlocking { val result = client.linkIdentity(connectionId = "conn-1") assertTrue(result.isFailure) @@ -124,7 +124,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `linkIdentity posts linkIdentity true and stores the new pending request`() = runTest { + fun `linkIdentity posts linkIdentity true and stores the new pending request`() = runBlocking { signIn() server.enqueue( MockResponse().setResponseCode(200).setBody( @@ -150,7 +150,7 @@ class AuthressLoginClientSessionTest { // ── getUserProfile() ───────────────────────────────────────────────── @Test - fun `getUserProfile fails when not signed in`() = runTest { + fun `getUserProfile fails when not signed in`() = runBlocking { val result = client.getUserProfile() assertTrue(result.isFailure) @@ -158,7 +158,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `getUserProfile returns the profile payload when signed in`() = runTest { + fun `getUserProfile returns the profile payload when signed in`() = runBlocking { signIn() server.enqueue(MockResponse().setResponseCode(200).setBody("""{"name":"Alex"}""")) @@ -172,7 +172,7 @@ class AuthressLoginClientSessionTest { // ── getDevices() ───────────────────────────────────────────────────── @Test - fun `getDevices returns empty without a network call when not signed in`() = runTest { + fun `getDevices returns empty without a network call when not signed in`() = runBlocking { val result = client.getDevices() assertTrue(result.isSuccess) @@ -181,7 +181,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `getDevices returns empty on a 401`() = runTest { + fun `getDevices returns empty on a 401`() = runBlocking { signIn() server.enqueue(MockResponse().setResponseCode(401)) @@ -192,7 +192,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `getDevices returns empty on a 404`() = runTest { + fun `getDevices returns empty on a 404`() = runBlocking { signIn() server.enqueue(MockResponse().setResponseCode(404)) @@ -203,7 +203,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `getDevices surfaces a genuine server error`() = runTest { + fun `getDevices surfaces a genuine server error`() = runBlocking { signIn() server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) @@ -213,7 +213,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `getDevices parses the device list on success`() = runTest { + fun `getDevices parses the device list on success`() = runBlocking { signIn() val devices = JSONArray() .put(JSONObject().put("deviceId", "dev-1").put("name", "Pixel")) @@ -234,7 +234,7 @@ class AuthressLoginClientSessionTest { // ── deleteDevice() ─────────────────────────────────────────────────── @Test - fun `deleteDevice calls the device-scoped delete endpoint`() = runTest { + fun `deleteDevice calls the device-scoped delete endpoint`() = runBlocking { server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) val result = client.deleteDevice("dev-1") @@ -246,7 +246,7 @@ class AuthressLoginClientSessionTest { } @Test - fun `deleteDevice surfaces a server failure`() = runTest { + fun `deleteDevice surfaces a server failure`() = runBlocking { server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) val result = client.deleteDevice("dev-1") diff --git a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt index 2a66996..acb7fe9 100644 --- a/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt +++ b/app/src/test/java/ch/rhosys/email/data/auth/AuthressLoginClientTokenTest.kt @@ -6,7 +6,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain -import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.setMain import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -136,7 +136,7 @@ class AuthressLoginClientTokenTest { // ── userIsLoggedIn() ───────────────────────────────────────────────── @Test - fun `userIsLoggedIn returns true without a network call when a valid token is cached`() = runTest { + fun `userIsLoggedIn returns true without a network call when a valid token is cached`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = 3600) assertTrue(client.userIsLoggedIn()) @@ -145,7 +145,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `userIsLoggedIn refreshes via PATCH slash session when the cached token is expired`() = runTest { + fun `userIsLoggedIn refreshes via PATCH slash session when the cached token is expired`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) server.enqueue( @@ -166,7 +166,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `userIsLoggedIn refreshes when there is no cookie at all yet`() = runTest { + fun `userIsLoggedIn refreshes when there is no cookie at all yet`() = runBlocking { val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) server.enqueue( MockResponse() @@ -180,7 +180,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `userIsLoggedIn returns false when the server rejects the refresh`() = runTest { + fun `userIsLoggedIn returns false when the server rejects the refresh`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) server.enqueue(MockResponse().setResponseCode(401).setBody("{\"error\":\"invalid session\"}")) @@ -190,7 +190,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `userIsLoggedIn returns false on a 500 from session refresh`() = runTest { + fun `userIsLoggedIn returns false on a 500 from session refresh`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) server.enqueue(MockResponse().setResponseCode(500).setBody("boom")) @@ -198,7 +198,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `userIsLoggedIn returns false on a network failure talking to the server`() = runTest { + fun `userIsLoggedIn returns false on a network failure talking to the server`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) server.shutdown() @@ -206,7 +206,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `userIsLoggedIn returns false when the refresh succeeds but the new cookie is still expired`() = runTest { + fun `userIsLoggedIn returns false when the refresh succeeds but the new cookie is still expired`() = runBlocking { // Pathological but should not be reported as logged in: the server // handed back a cookie that is already expired by our clock. cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) @@ -224,7 +224,7 @@ class AuthressLoginClientTokenTest { // ── waitForToken() ─────────────────────────────────────────────────── @Test - fun `waitForToken returns the cached token immediately without a network call`() = runTest { + fun `waitForToken returns the cached token immediately without a network call`() = runBlocking { val token = testJwt(TEST_ORIGIN, secondsFromNow = 3600) cookieBacking.cookies["authorization"] = token @@ -233,7 +233,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `waitForToken with a zero timeout returns null immediately without refreshing`() = runTest { + fun `waitForToken with a zero timeout returns null immediately without refreshing`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) assertNull(client.waitForToken(timeoutInMillis = 0)) @@ -241,7 +241,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `waitForToken refreshes an expired token and returns the new one`() = runTest { + fun `waitForToken refreshes an expired token and returns the new one`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) val freshToken = testJwt(TEST_ORIGIN, secondsFromNow = 3600) server.enqueue( @@ -255,7 +255,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `waitForToken returns null promptly when the refresh is rejected, without waiting out the full timeout`() = runTest { + fun `waitForToken returns null promptly when the refresh is rejected, without waiting out the full timeout`() = runBlocking { cookieBacking.cookies["authorization"] = testJwt(TEST_ORIGIN, secondsFromNow = -60) server.enqueue(MockResponse().setResponseCode(401)) @@ -268,7 +268,7 @@ class AuthressLoginClientTokenTest { } @Test - fun `waitForToken on a hung server fails within OkHttp's own read timeout, bounding wall time`() = runTest { + fun `waitForToken on a hung server fails within OkHttp's own read timeout, bounding wall time`() = runBlocking { // withTimeoutOrNull cannot interrupt a synchronous OkHttp Call.execute() // mid-flight, so what actually bounds a hung PATCH /session here is the // client's own read timeout, not waitForToken's timeoutInMillis. This diff --git a/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt b/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt index f63bb4d..caf995a 100644 --- a/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt +++ b/app/src/test/java/ch/rhosys/email/data/auth/JwtManagerTest.kt @@ -1,7 +1,7 @@ package ch.rhosys.email.data.auth import ch.rhosys.email.testutil.testJwt -import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.runBlocking import org.json.JSONObject import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -82,7 +82,7 @@ class JwtManagerTest { } @Test - fun `calculateAntiAbuseHash produces the v2 format with a hash starting 00`() = runTest { + fun `calculateAntiAbuseHash produces the v2 format with a hash starting 00`() = runBlocking { val hash = JwtManager.calculateAntiAbuseHash(linkedMapOf("applicationId" to "app-1")) val parts = hash.split(";") assertEquals(4, parts.size) @@ -93,7 +93,7 @@ class JwtManagerTest { } @Test - fun `calculateAntiAbuseHash ignores null, empty-string and false values`() = runTest { + fun `calculateAntiAbuseHash ignores null, empty-string and false values`() = runBlocking { // Sanity check that it doesn't throw and still produces a valid hash // when most props are absent, mirroring authenticate()'s default options. val hash = JwtManager.calculateAntiAbuseHash( diff --git a/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt b/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt index 8e6809e..9eda6fd 100644 --- a/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt +++ b/app/src/test/java/ch/rhosys/email/data/realtime/RealtimeClientTest.kt @@ -45,7 +45,12 @@ class RealtimeClientTest { @After fun tearDown() { - server.shutdown() + // A WebSocket's close handshake (triggered by client.stop() in each test) + // is asynchronous; MockWebServer.shutdown() can throw IOException if it + // still sees that connection as open when called immediately after. + // Harmless here — the JVM tears down these sockets/threads regardless, + // and every test's own assertions already ran before this executes. + runCatching { server.shutdown() } } private fun wsBaseUrl(): String = server.url("/").toString().trimEnd('/')