From 866845fbc9575e355757c18ecf4ead39c8243f4d Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Thu, 23 Jul 2026 16:12:06 +0100 Subject: [PATCH 1/3] [SDK-547] Synchronize JWT auth refresh timer scheduling The refresh timer, app foreground, and 401 retry paths all call scheduleAuthTokenRefresh from different threads. The isTimerScheduled guard was a non-atomic check-then-act: concurrent callers could all pass it and each schedule a TimerTask, and each foreground reschedule could create a new Timer while orphaning the previous one. Orphaned timers could not be cancelled by clearRefreshTimer and kept firing onAuthTokenRequested, inflating backend JWT generation over time. Make scheduleAuthTokenRefresh and clearRefreshTimer synchronized, and set isTimerScheduled before scheduling (reset on failure) so only one refresh timer is ever active. Adds a concurrency test asserting a single timer under 8 racing callers, and a test asserting foregrounding with a valid, far-from-expiry token does not request a new token. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 + .../iterableapi/IterableAuthManager.java | 13 ++- .../iterableapi/IterableApiAuthTests.java | 88 +++++++++++++++++++ 3 files changed, 99 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d40ce8f5..d254f304c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed +- Fixed a race in JWT auth token refresh scheduling that could leave multiple overlapping refresh timers running. When the refresh timer, an app foreground, and a 401 retry raced to schedule a refresh, the non-atomic timer guard let each create its own timer; the orphaned timers could not be cancelled and each kept requesting new auth tokens, inflating the number of `IterableAuthHandler.onAuthTokenRequested()` calls (and backend JWT generation) over time. Scheduling and clearing of the refresh timer are now synchronized so only one refresh timer is ever active. ## [3.10.0] ### Added diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index 3a960e54a..73bb9b3af 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -291,7 +291,7 @@ long getNextRetryInterval() { return nextRetryInterval; } - void scheduleAuthTokenRefresh(long timeDuration, boolean isScheduledRefresh, final IterableHelper.SuccessHandler successCallback) { + synchronized void scheduleAuthTokenRefresh(long timeDuration, boolean isScheduledRefresh, final IterableHelper.SuccessHandler successCallback) { if ((pauseAuthRetry && !isScheduledRefresh) || isTimerScheduled) { // we only stop schedule token refresh if it is called from retry (in case of failure). The normal auth token refresh schedule would work return; @@ -301,6 +301,9 @@ void scheduleAuthTokenRefresh(long timeDuration, boolean isScheduledRefresh, fin } try { + // Set the flag before scheduling so concurrent callers can't pass the guard above and + // orphan a second timer (SDK-547). + isTimerScheduled = true; timer.schedule(new TimerTask() { @Override public void run() { @@ -309,11 +312,13 @@ public void run() { } else { IterableLogger.w(TAG, "Email or userId is not available. Skipping token refresh"); } - isTimerScheduled = false; + synchronized (IterableAuthManager.this) { + isTimerScheduled = false; + } } }, timeDuration); - isTimerScheduled = true; } catch (Exception e) { + isTimerScheduled = false; IterableLogger.e(TAG, "timer exception: " + timer, e); } } @@ -362,7 +367,7 @@ private void checkAndHandleAuthRefresh() { } } - void clearRefreshTimer() { + synchronized void clearRefreshTimer() { if (timer != null) { timer.cancel(); timer = null; diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java index ebc879508..be1361079 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java @@ -11,7 +11,10 @@ import java.io.IOException; import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.CyclicBarrier; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; @@ -23,8 +26,11 @@ import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertNull; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.robolectric.Shadows.shadowOf; import static org.robolectric.annotation.LooperMode.Mode.PAUSED; @@ -509,4 +515,86 @@ public void testAuthTokenRefreshPausesOnBackground() throws Exception { // Test passes if no exceptions were thrown and lifecycle methods executed successfully } + // SDK-547: foregrounding the app with a valid token that is far from expiry must NOT request a + // new token. onSwitchToForeground re-evaluates the token (a deliberate Android behavior, since + // java.util.Timer is unreliable across background/Doze) but should only re-arm the expiry timer, + // not call the developer's onAuthTokenRequested. Requesting on every foreground is what inflates + // backend JWT volume relative to iOS. + @Test + public void testForegroundWithValidTokenDoesNotRequestNewToken() throws Exception { + IterableApi.initialize(getContext(), "apiKey"); + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + + // Seed a valid token far from expiry (validJWT exp is year 2062) without going through + // onAuthTokenRequested. + IterableApi.getInstance().setEmail("test@example.com", validJWT); + shadowOf(getMainLooper()).runToEndOfTasks(); + assertEquals(validJWT, IterableApi.getInstance().getAuthToken()); + + // Ignore any handler interactions from setup; we only care about the foreground transition. + clearInvocations(authHandler); + + authManager.onSwitchToBackground(); + authManager.onSwitchToForeground(); + shadowOf(getMainLooper()).runToEndOfTasks(); + + verify(authHandler, never()).onAuthTokenRequested(); + } + + // SDK-547: scheduleAuthTokenRefresh reads isTimerScheduled, schedules a TimerTask, then sets + // isTimerScheduled=true only AFTER the schedule call returns. The read/set isn't atomic, so + // concurrent callers (foreground refresh, 401 retry, an already-firing timer) all observe + // false and each schedule their own timer off a single guard. Overlapping timers each fire + // onAuthTokenRequested and each success reschedules, so JWT request volume compounds. + // + // We drive scheduleAuthTokenRefresh directly rather than requestNewAuthToken: the executor is + // not injectable (see @Ignore'd tests above) and its pendingAuth guard serializes calls, + // which would hide the scheduling race we're targeting. + @Test + public void testConcurrentScheduleAuthTokenRefreshSchedulesOnlyOneTimer() throws Exception { + IterableApi.initialize(getContext(), "apiKey"); + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + + final AtomicInteger scheduleCount = new AtomicInteger(0); + // Fake timer that counts schedule() calls and holds briefly, so every racing thread has + // passed the guard before the winner writes isTimerScheduled=true. + Timer countingTimer = new Timer(true) { + @Override + public void schedule(TimerTask task, long delay) { + scheduleCount.incrementAndGet(); + try { + Thread.sleep(100); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + }; + authManager.timer = countingTimer; + + final int threadCount = 8; + final CyclicBarrier barrier = new CyclicBarrier(threadCount); + Thread[] threads = new Thread[threadCount]; + for (int i = 0; i < threadCount; i++) { + threads[i] = new Thread(() -> { + try { + barrier.await(); + authManager.scheduleAuthTokenRefresh(60000, true, null); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + for (Thread t : threads) { + t.start(); + } + for (Thread t : threads) { + t.join(); + } + + countingTimer.cancel(); + + assertEquals("Concurrent scheduling should result in exactly one live timer", + 1, scheduleCount.get()); + } + } From 2198b809e004d5845c9d68d1053f566af94f6923 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Mon, 27 Jul 2026 18:57:08 +0100 Subject: [PATCH 2/3] [SDK-547] Don't wipe credentials on transient crypto timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IterableKeychain decrypts/encrypts the stored auth token under a 500ms timeout. A slow AndroidKeyStore operation that exceeded it was caught in the same branch as a genuine decryption failure, which wipes the stored email, userId, and auth token and permanently disables encryption. That forces a re-login and a fresh onAuthTokenRequested on the next launch — on slower devices this can recur, inflating backend JWT generation. Handle TimeoutException separately from real crypto failures: on a timeout, preserve the encrypted data and fall back only for the current read/write (plaintext on save), without wiping credentials or disabling encryption. Genuine decryption errors still wipe as before. Adds a test asserting a crypto timeout does not wipe credentials, disable encryption, or invoke the decryption-failure handler. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + .../iterable/iterableapi/IterableKeychain.kt | 16 +++++++++++++ .../iterableapi/IterableKeychainTest.kt | 24 +++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d254f304c..a9b52d23c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Fixed - Fixed a race in JWT auth token refresh scheduling that could leave multiple overlapping refresh timers running. When the refresh timer, an app foreground, and a 401 retry raced to schedule a refresh, the non-atomic timer guard let each create its own timer; the orphaned timers could not be cancelled and each kept requesting new auth tokens, inflating the number of `IterableAuthHandler.onAuthTokenRequested()` calls (and backend JWT generation) over time. Scheduling and clearing of the refresh timer are now synchronized so only one refresh timer is ever active. +- Fixed the keychain treating a transient crypto timeout as a permanent decryption failure. A slow AndroidKeyStore operation that exceeded the 500 ms timeout would wipe the stored email, userId, and auth token and disable encryption, forcing the user to re-authenticate (and request a new auth token) on the next launch. Crypto timeouts are now handled as transient: the encrypted data is preserved and only the current read/write falls back, without wiping credentials or disabling encryption. ## [3.10.0] ### Added diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt index 1a8235b09..0c29fe8b2 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt @@ -4,6 +4,7 @@ import android.content.Context import android.content.SharedPreferences import java.util.concurrent.Callable import java.util.concurrent.Executors +import java.util.concurrent.TimeoutException import java.util.concurrent.TimeUnit class IterableKeychain { @@ -120,6 +121,13 @@ class IterableKeychain { val encryptedValue = sharedPrefs.getString(key, null) ?: return null return try { encryptor?.let { runWithTimeout { it.decrypt(encryptedValue) } } + } catch (e: TimeoutException) { + // A crypto operation that times out is transient (slow/contended AndroidKeyStore), not a + // corrupt key. Don't wipe stored credentials or disable encryption over it — that would + // force a re-login (and a new auth-token request) on every slow launch. Return null for + // this read; the encrypted value stays intact for the next attempt. (SDK-547) + IterableLogger.w(TAG, "Crypto operation timed out; keeping encrypted data for retry.") + null } catch (e: Exception) { handleDecryptionError(e) null @@ -145,6 +153,14 @@ class IterableKeychain { .remove(key + PLAINTEXT_SUFFIX) .apply() } + } catch (e: TimeoutException) { + // Transient slow crypto: store this value as plaintext so it isn't lost, but don't wipe + // other credentials or disable encryption globally. Encryption stays on for future + // writes. (SDK-547) + IterableLogger.w(TAG, "Crypto operation timed out on save; storing this value as plaintext.") + editor.putString(key, value) + .putBoolean(key + PLAINTEXT_SUFFIX, true) + .apply() } catch (e: Exception) { handleDecryptionError(e) editor.putString(key, value) diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt index 53fc5944b..f6258ee13 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt @@ -178,6 +178,30 @@ class IterableKeychainTest { assertNull(result) } + @Test + fun testDecryptionTimeoutDoesNotWipeCredentials() { + // SDK-547: a transient crypto timeout (slow AndroidKeyStore) must NOT wipe stored + // credentials or disable encryption — otherwise the app re-logs-in (and requests a new + // auth token) on every slow launch. Simulate a slow decrypt that exceeds the 500ms timeout. + `when`(mockEncryptor.decrypt(any())).thenAnswer { + Thread.sleep(700) + "should_not_be_returned" + } + `when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())) + .thenReturn("any_encrypted_value") + + val result = keychain.getAuthToken() + + // Read returns null for this attempt... + assertNull(result) + // ...but nothing is wiped and the failure handler is NOT invoked (it's transient). + verify(mockEditor, never()).remove("iterable-email") + verify(mockEditor, never()).remove("iterable-user-id") + verify(mockEditor, never()).remove("iterable-auth-token") + verify(mockEditor, never()).putBoolean(eq("iterable-encryption-enabled"), eq(false)) + verify(mockDecryptionFailureHandler, never()).onDecryptionFailed(any()) + } + @Test fun testDecryptionFailureForAllOperations() { // Setup mock to throw runtime exception From f196b97aa718d12cff47371604aafe17b951d6df Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Tue, 28 Jul 2026 09:56:49 +0100 Subject: [PATCH 3/3] [SDK-547] Cancel timed-out crypto op so it can't block later reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. runWithTimeout ran crypto on a single-thread executor and, on timeout, left the Future running — a slow/hung AndroidKeyStore operation kept occupying the only worker thread, so every subsequent read/write queued behind it and also timed out. Cancel the Future on timeout (interrupting the task if interruptible) to free the thread. Also clarify the CHANGELOG to note that a write timeout stores that one value unencrypted (the existing non-encrypted fallback), rather than implying nothing is written. Adds a test asserting a slow crypto op that times out does not block the next read (fails without the cancel). Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- .../iterable/iterableapi/IterableKeychain.kt | 10 ++++++- .../iterableapi/IterableKeychainTest.kt | 27 +++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9b52d23c..644799d60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Fixed - Fixed a race in JWT auth token refresh scheduling that could leave multiple overlapping refresh timers running. When the refresh timer, an app foreground, and a 401 retry raced to schedule a refresh, the non-atomic timer guard let each create its own timer; the orphaned timers could not be cancelled and each kept requesting new auth tokens, inflating the number of `IterableAuthHandler.onAuthTokenRequested()` calls (and backend JWT generation) over time. Scheduling and clearing of the refresh timer are now synchronized so only one refresh timer is ever active. -- Fixed the keychain treating a transient crypto timeout as a permanent decryption failure. A slow AndroidKeyStore operation that exceeded the 500 ms timeout would wipe the stored email, userId, and auth token and disable encryption, forcing the user to re-authenticate (and request a new auth token) on the next launch. Crypto timeouts are now handled as transient: the encrypted data is preserved and only the current read/write falls back, without wiping credentials or disabling encryption. +- Fixed the keychain treating a transient crypto timeout as a permanent decryption failure. A slow AndroidKeyStore operation that exceeded the 500 ms timeout would wipe the stored email, userId, and auth token and disable encryption, forcing the user to re-authenticate (and request a new auth token) on the next launch. Crypto timeouts are now handled as transient without wiping credentials or disabling encryption for the device: a read that times out returns no value for that call (the stored ciphertext is left intact for the next attempt), and a write that times out stores that one value unencrypted (as the non-encrypted fallback already did) rather than clearing everything. The timed-out crypto operation is also cancelled so it no longer blocks subsequent reads/writes. ## [3.10.0] ### Added diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt index 0c29fe8b2..1d5b22ba0 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt @@ -72,7 +72,15 @@ class IterableKeychain { } private fun runWithTimeout(callable: Callable): T { - return cryptoExecutor.submit(callable).get(CRYPTO_OPERATION_TIMEOUT_MS, TimeUnit.MILLISECONDS) + val future = cryptoExecutor.submit(callable) + try { + return future.get(CRYPTO_OPERATION_TIMEOUT_MS, TimeUnit.MILLISECONDS) + } catch (e: Exception) { + // Free the single crypto thread so a slow/hung operation doesn't block every subsequent + // read/write behind it. cancel(true) interrupts the task if it's interruptible. (SDK-547) + future.cancel(true) + throw e + } } private fun handleDecryptionError(e: Exception? = null) { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt index f6258ee13..3d0ccc913 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt @@ -202,6 +202,33 @@ class IterableKeychainTest { verify(mockDecryptionFailureHandler, never()).onDecryptionFailed(any()) } + @Test + fun testCryptoTimeoutDoesNotBlockSubsequentReads() { + // SDK-547: crypto runs on a single-thread executor. A slow op that times out must be + // cancelled so it frees the thread and doesn't clog the next read. Here the first decrypt + // is interruptible-sleeping past the 500ms timeout; the second is fast. Without cancelling + // the timed-out task, the second read would queue behind the still-running first one and + // also time out (null). With cancellation it completes normally. + val firstCall = java.util.concurrent.atomic.AtomicBoolean(true) + `when`(mockEncryptor.decrypt(any())).thenAnswer { + if (firstCall.getAndSet(false)) { + Thread.sleep(5000) // slow/hung; will be interrupted by cancel(true) + "slow_value" + } else { + "encrypted_fast".substring("encrypted_".length) // -> "fast" + } + } + `when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())) + .thenReturn("any_encrypted_value") + `when`(mockSharedPrefs.getString(eq("iterable-email"), isNull())) + .thenReturn("encrypted_fast") + + // First read times out -> null (task gets cancelled/interrupted, freeing the thread). + assertNull(keychain.getAuthToken()) + // Second read must NOT be blocked behind the first; it decrypts promptly. + assertEquals("fast", keychain.getEmail()) + } + @Test fun testDecryptionFailureForAllOperations() { // Setup mock to throw runtime exception