[SDK-547] Reduce excess JWT auth token requests (timer race + crypto-timeout wipe)

CHANGELOG.md

@@ -3,6 +3,9 @@ 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.
+- 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

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;

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)

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

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