From 29c12de84197bc0ba7ce4833d3b18918f7f26e62 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Thu, 23 Jul 2026 16:12:06 +0100 Subject: [PATCH 01/11] [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 bc98cafa198ecf262ca4d64c5f3d89c2d34c59e3 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Mon, 27 Jul 2026 18:57:08 +0100 Subject: [PATCH 02/11] [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 b587b29734a3730152831f0f17f3b9910c8ab3c3 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Tue, 28 Jul 2026 09:56:49 +0100 Subject: [PATCH 03/11] [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 From 4b9cb962074fdf5744bd06a9123de84d5dff8868 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Tue, 18 Aug 2026 09:45:57 +0100 Subject: [PATCH 04/11] [SDK-547] Cover auth token lifecycle with deterministic tests The auth manager submitted token requests to a privately constructed executor, so tests could not observe or order them; 17 tests in IterableApiAuthTests are @Ignore'd for exactly this reason. Make the executor injectable so a test can drain it deliberately. Adds 20 tests covering token creation, replacement, recovery after failure, foreground/background transitions, and concurrent scheduling (the regression net for the timer race fixed in 866845fb). The tests queue work instead of running it inside submit(): the SDK holds the auth manager's monitor across executor.submit(), and the submitted task re-enters that monitor via queueExpirationRefresh, so any same-thread executor deadlocks. Co-Authored-By: Claude Opus 5 --- .../iterableapi/IterableAuthManager.java | 3 +- .../IterableAuthTokenLifecycleTest.java | 476 ++++++++++++++++++ 2 files changed, 478 insertions(+), 1 deletion(-) create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index 73bb9b3af..c6db12ef7 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -57,7 +57,8 @@ interface AuthTokenReadyListener { private volatile AuthState authState = AuthState.UNKNOWN; private final ArrayList authTokenReadyListeners = new ArrayList<>(); - private final ExecutorService executor = Executors.newSingleThreadExecutor(); + @VisibleForTesting + ExecutorService executor = Executors.newSingleThreadExecutor(); IterableAuthManager(IterableApi api, IterableAuthHandler authHandler, RetryPolicy authRetryPolicy, long expiringAuthTokenRefreshPeriod) { this.api = api; diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java new file mode 100644 index 000000000..fb04c2951 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java @@ -0,0 +1,476 @@ +package com.iterable.iterableapi; + +import com.iterable.iterableapi.unit.PathBasedQueueDispatcher; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.robolectric.annotation.LooperMode; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import okhttp3.mockwebserver.MockWebServer; + +import static android.os.Looper.getMainLooper; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.robolectric.Shadows.shadowOf; +import static org.robolectric.annotation.LooperMode.Mode.PAUSED; + +/** + * Covers how the JWT auth token is created, replaced and recovered as the refresh timer, + * foreground/background transitions and login/logout drive {@link IterableAuthManager}. + */ +@LooperMode(PAUSED) +public class IterableAuthTokenLifecycleTest extends BaseTest { + + /** exp = 2062. */ + private static final String VALID_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9.mYtgSqdUIxK8_RnYBTUP4cmpKw83aKi7cMiixF3qMB4"; + /** exp = 2030. */ + private static final String NEW_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE5MTYyMzkwMjJ9.dMD3MLuHTiO-Qy9PvOoMchNM4CzFIgI7jKVrRtlqlM0"; + /** exp = 2018, already expired. */ + private static final String EXPIRED_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE1MTYyNDkwMjJ9.6Yc3QcBGwCdV1sdKmgOtw4D69P_HUoVqEW3YMuEgH8c"; + + private static final String EMAIL = "user@example.com"; + private static final String OTHER_EMAIL = "other@example.com"; + + private MockWebServer server; + private IterableAuthHandler authHandler; + private IterableAuthManager authManager; + private ManualExecutor executor; + + @Before + public void setUp() { + server = new MockWebServer(); + server.setDispatcher(new PathBasedQueueDispatcher()); + IterableApi.overrideURLEndpointPath(server.url("").toString()); + + IterableApi.sharedInstance = new IterableApi(); + authHandler = mock(IterableAuthHandler.class); + doReturn(VALID_JWT).when(authHandler).onAuthTokenRequested(); + + IterableApi.initialize(getContext(), "apiKey", new IterableConfig.Builder() + .setAutoPushRegistration(false) + .setAuthHandler(authHandler) + .build()); + + authManager = IterableApi.getInstance().getAuthManager(); + executor = new ManualExecutor(); + authManager.executor = executor; + } + + @After + public void tearDown() throws IOException { + executor.shutdownNow(); + server.shutdown(); + server = null; + } + + // region token creation + + @Test + public void loginRequestsTokenFromHandlerAndStoresIt() { + login(); + + verify(authHandler).onAuthTokenRequested(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void loginWithSuppliedTokenDoesNotAskTheHandler() { + IterableApi.getInstance().setEmail(EMAIL, VALID_JWT); + settle(); + + verify(authHandler, never()).onAuthTokenRequested(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void handlerReturningNullLeavesNoTokenStored() { + doReturn(null).when(authHandler).onAuthTokenRequested(); + + login(); + + assertNull(IterableApi.getInstance().getAuthToken()); + verify(authHandler).onAuthFailure(failureWithReason(AuthFailureReason.AUTH_TOKEN_NULL)); + } + + @Test + public void handlerThrowingLeavesNoTokenStored() { + doThrow(new RuntimeException("backend down")).when(authHandler).onAuthTokenRequested(); + + login(); + + assertNull(IterableApi.getInstance().getAuthToken()); + verify(authHandler).onAuthFailure(failureWithReason(AuthFailureReason.AUTH_TOKEN_GENERATION_ERROR)); + } + + // endregion + + // region token replacement + + @Test + public void switchingUserReplacesTheStoredToken() { + login(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + + doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); + loginAs(OTHER_EMAIL); + + assertEquals(NEW_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void reLoggingInAsTheSameUserKeepsTheStoredToken() { + login(); + clearInvocations(authHandler); + + doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); + login(); + + verify(authHandler, never()).onAuthTokenRequested(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void failedRefreshKeepsThePreviousToken() { + login(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + + doThrow(new RuntimeException("backend down")).when(authHandler).onAuthTokenRequested(); + authManager.requestNewAuthToken(false, null); + settle(); + + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void logoutClearsTheToken() { + login(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + + IterableApi.getInstance().setEmail(null); + settle(); + + assertNull(IterableApi.getInstance().getAuthToken()); + assertNull(IterableApi.getInstance().getEmail()); + } + + // endregion + + // region recovery after failure + + @Test + public void refreshAfterAFailureRecoversTheToken() { + doThrow(new RuntimeException("backend down")).when(authHandler).onAuthTokenRequested(); + login(); + assertNull(IterableApi.getInstance().getAuthToken()); + + doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); + authManager.requestNewAuthToken(false, null); + settle(); + + assertEquals(NEW_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void expiredTokenIsStoredAndRefreshIsRescheduled() { + doReturn(EXPIRED_JWT).when(authHandler).onAuthTokenRequested(); + + login(); + + assertEquals(EXPIRED_JWT, IterableApi.getInstance().getAuthToken()); + assertTrue("an expired token must leave a refresh armed", isRefreshScheduled()); + } + + @Test + public void malformedTokenReportsPayloadInvalidAndKeepsRefreshing() { + doReturn("not.a.jwt").when(authHandler).onAuthTokenRequested(); + + login(); + + verify(authHandler).onAuthFailure(failureWithReason(AuthFailureReason.AUTH_TOKEN_PAYLOAD_INVALID)); + assertTrue(isRefreshScheduled()); + } + + // endregion + + // region foreground / background + + @Test + public void foregroundWithAValidTokenDoesNotRequestANewOne() { + login(); + clearInvocations(authHandler); + + authManager.onSwitchToBackground(); + authManager.onSwitchToForeground(); + settle(); + + verify(authHandler, never()).onAuthTokenRequested(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void backgroundCancelsTheScheduledRefresh() { + login(); + assertTrue(isRefreshScheduled()); + + authManager.onSwitchToBackground(); + + assertNull("backgrounding must cancel the refresh timer", authManager.timer); + } + + @Test + public void repeatedForegroundingDoesNotAmplifyTokenRequests() { + login(); + clearInvocations(authHandler); + + for (int i = 0; i < 5; i++) { + authManager.onSwitchToForeground(); + } + settle(); + + verify(authHandler, never()).onAuthTokenRequested(); + assertTrue("foregrounding must leave exactly one refresh armed", isRefreshScheduled()); + } + + @Test + public void tokenRequestIsSkippedWhileBackgrounded() { + login(); + clearInvocations(authHandler); + + authManager.onSwitchToBackground(); + authManager.requestNewAuthToken(false, null); + settle(); + + verify(authHandler, never()).onAuthTokenRequested(); + } + + // endregion + + // region concurrent actors + + @Test + public void concurrentSchedulingArmsOnlyOneRefresh() throws Exception { + CountingTimer timer = installCountingTimer(); + + runConcurrently(8, () -> authManager.scheduleAuthTokenRefresh(60_000, true, null)); + + assertEquals(1, timer.liveTaskCount()); + } + + @Test + public void aFiringRefreshRequestsOneTokenAndRearmsOnce() { + login(); + clearInvocations(authHandler); + + CountingTimer timer = armObservableRefresh(); + timer.fireAll(); + settle(); + + verify(authHandler, times(1)).onAuthTokenRequested(); + assertTrue("the refreshed token must leave a new refresh armed", isRefreshScheduled()); + } + + @Test + public void concurrentTokenRequestsCallTheHandlerOnce() throws Exception { + login(); + clearInvocations(authHandler); + + runConcurrently(8, () -> authManager.requestNewAuthToken(false, null)); + settle(); + + verify(authHandler, times(1)).onAuthTokenRequested(); + } + + @Test + public void loginDuringAnInFlightRequestReusesThatRequest() { + authManager.requestNewAuthToken(false, null); + + doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); + login(); + + verify(authHandler, times(1)).onAuthTokenRequested(); + assertEquals(NEW_JWT, IterableApi.getInstance().getAuthToken()); + } + + @Test + public void tokenArrivingAfterLogoutIsNotStored() { + login(); + + authManager.requestNewAuthToken(false, null); + IterableApi.getInstance().setEmail(null); + settle(); + + assertNull("a token resolved after logout must not restore the session", + IterableApi.getInstance().getAuthToken()); + } + + // endregion + + private void login() { + loginAs(EMAIL); + } + + private void loginAs(String email) { + IterableApi.getInstance().setEmail(email); + settle(); + } + + /** Drains the auth executor and the main looper until both are idle. */ + private void settle() { + for (int i = 0; i < 10; i++) { + boolean ranTask = executor.runAll() > 0; + shadowOf(getMainLooper()).runToEndOfTasks(); + if (!ranTask && !executor.hasPendingTasks()) { + return; + } + } + } + + private boolean isRefreshScheduled() { + return authManager.timer != null; + } + + private CountingTimer installCountingTimer() { + CountingTimer timer = new CountingTimer(); + authManager.timer = timer; + return timer; + } + + /** Discards whatever refresh is already armed and arms one the test can fire on demand. */ + private CountingTimer armObservableRefresh() { + authManager.clearRefreshTimer(); + CountingTimer timer = installCountingTimer(); + authManager.scheduleAuthTokenRefresh(60_000, true, null); + assertEquals(1, timer.liveTaskCount()); + return timer; + } + + private void runConcurrently(int threadCount, Runnable action) throws Exception { + CountDownLatch start = new CountDownLatch(1); + CountDownLatch done = new CountDownLatch(threadCount); + for (int i = 0; i < threadCount; i++) { + new Thread(() -> { + try { + start.await(); + action.run(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + done.countDown(); + } + }).start(); + } + start.countDown(); + assertTrue(done.await(10, TimeUnit.SECONDS)); + } + + private static AuthFailure failureWithReason(AuthFailureReason reason) { + return org.mockito.ArgumentMatchers.argThat(failure -> failure != null && failure.failureReason == reason); + } + + /** Records scheduled tasks instead of running them, so a test can count and fire them. */ + private static class CountingTimer extends Timer { + private final List tasks = new CopyOnWriteArrayList<>(); + + CountingTimer() { + super(true); + super.cancel(); + } + + @Override + public void schedule(TimerTask task, long delay) { + tasks.add(task); + } + + @Override + public void cancel() { + tasks.clear(); + } + + int liveTaskCount() { + return tasks.size(); + } + + void fireAll() { + for (TimerTask task : tasks) { + task.run(); + } + } + } + + /** + * Executor that queues submitted work until the test drains it. Unlike Robolectric's + * InlineExecutorService this never runs the task inside submit(), which would deadlock: + * requestNewAuthToken submits while holding the auth manager's monitor, and the task + * re-enters that monitor via queueExpirationRefresh. + */ + private static class ManualExecutor extends java.util.concurrent.AbstractExecutorService { + private final java.util.Queue pending = new java.util.concurrent.ConcurrentLinkedQueue<>(); + private volatile boolean shutdown; + + @Override + public void execute(Runnable command) { + if (!shutdown) { + pending.add(command); + } + } + + int runAll() { + int count = 0; + Runnable task; + while ((task = pending.poll()) != null) { + task.run(); + count++; + } + return count; + } + + boolean hasPendingTasks() { + return !pending.isEmpty(); + } + + @Override + public void shutdown() { + shutdown = true; + } + + @Override + public List shutdownNow() { + shutdown = true; + pending.clear(); + return Collections.emptyList(); + } + + @Override + public boolean isShutdown() { + return shutdown; + } + + @Override + public boolean isTerminated() { + return shutdown && pending.isEmpty(); + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) { + return isTerminated(); + } + } +} From c9086774b15475b28882514e8363a7c63cc7438c Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Tue, 18 Aug 2026 11:15:26 +0100 Subject: [PATCH 05/11] [SDK-547] Make JWT restore and refresh ownership explicit Keep timed-out token reads distinct from confirmed absence and retry them off the caller thread without minting a replacement token on exhausted reads. Track the pending refresh task itself, attach named reasons to refresh decisions, and prevent stale restore or timer work from overtaking identity changes. --- CHANGELOG.md | 5 +- .../com/iterable/iterableapi/IterableApi.java | 42 +-- .../iterableapi/IterableAuthDataRestorer.java | 216 +++++++++++++++ .../iterableapi/IterableAuthManager.java | 260 +++++++++++++++--- .../IterableAuthRefreshReason.java | 28 ++ .../iterable/iterableapi/IterableKeychain.kt | 43 ++- .../iterableapi/IterableRequestTask.java | 10 +- .../iterableapi/IterableApiAuthTests.java | 16 +- ...terableAuthDataRestoreIntegrationTest.java | 165 +++++++++++ .../IterableAuthDataRestorerTest.java | 130 +++++++++ .../IterableAuthRefreshOwnershipTest.java | 127 +++++++++ .../IterableAuthTokenLifecycleTest.java | 17 +- .../iterableapi/IterableKeychainTest.kt | 26 +- 13 files changed, 991 insertions(+), 94 deletions(-) create mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java create mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRefreshReason.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 644799d60..b9d225ee9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ 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 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. +- Fixed a race in JWT auth token refresh scheduling that could leave multiple overlapping refresh timers running. The pending refresh task is now the single source of ownership, and cancelled or replaced tasks cannot execute or clear their replacement. Refresh scheduling also records an explicit reason, such as token expiration, a 401 retry, or a missing stored token. +- 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 on the next launch. Crypto timeouts now preserve stored data and encryption state; timed-out operations are cancelled so they do not block later reads or writes. +- Fixed a timed-out stored-token read being mistaken for a confirmed missing JWT. Transient token-read timeouts are retried off the caller thread, and `IterableAuthHandler.onAuthTokenRequested()` is invoked only after a completed read confirms that the stored token is absent. If storage remains unavailable, JWT-required work stays blocked and restoration is retried when the app returns to the foreground. ## [3.10.0] ### Added diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index 95eb354ba..a24985088 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -22,6 +22,7 @@ import java.util.Objects; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; /** * Created by David Truong dt@iterable.com @@ -38,7 +39,7 @@ public class IterableApi { private String _email; private String _userId; String _userIdUnknown; - private String _authToken; + private volatile String _authToken; private boolean _debugMode; private Bundle _payloadData; private IterableNotificationData _notificationData; @@ -59,9 +60,8 @@ public class IterableApi { private String inboxSessionId; private IterableAuthManager authManager; private ConcurrentHashMap deviceAttributes = new ConcurrentHashMap<>(); - private IterableKeychain keychain; - - + @VisibleForTesting IterableKeychain keychain; + @VisibleForTesting ScheduledExecutorService authDataRestoreExecutor; //region Background Initialization - Delegated to IterableBackgroundInitializer //--------------------------------------------------------------------------------------- @@ -138,9 +138,8 @@ public String getAuthToken() { } private void checkAndUpdateAuthToken(@Nullable String authToken) { - // If authHandler exists and if authToken is new, it will be considered as a call to update the authToken. if (config.authHandler != null && authToken != null && authToken != _authToken) { - setAuthToken(authToken); + getAuthManager().useExplicitAuthToken(authToken); } } @@ -410,7 +409,7 @@ private void logoutPreviousUser() { embeddedManager.reset(); } if (authManager != null) { - authManager.reset(); + authManager.resetForIdentityChange(); } if (apiClient != null) { @@ -612,24 +611,21 @@ private void retrieveEmailAndUserId() { if (_applicationContext == null) { return; } + IterableKeychain iterableKeychain = getKeychain(); - if (iterableKeychain != null) { - _email = iterableKeychain.getEmail(); - _userId = iterableKeychain.getUserId(); - _userIdUnknown = iterableKeychain.getUserIdUnknown(); - _authToken = iterableKeychain.getAuthToken(); - } else { + if (iterableKeychain == null) { IterableLogger.e(TAG, "retrieveEmailAndUserId: Shared preference creation failed. Could not retrieve email/userId"); + return; } + _email = iterableKeychain.getEmail(); + _userId = iterableKeychain.getUserId(); + _userIdUnknown = iterableKeychain.getUserIdUnknown(); - if (config.authHandler != null && checkSDKInitialization()) { - if (_authToken != null) { - getAuthManager().queueExpirationRefresh(_authToken); - } else { - IterableLogger.d(TAG, "Auth token found as null. Rescheduling auth token refresh"); - getAuthManager().scheduleAuthTokenRefresh(authManager.getNextRetryInterval(), true, null); - } + if (config.authHandler == null || !checkSDKInitialization()) { + _authToken = iterableKeychain.getAuthToken(); + return; } + _authToken = getAuthManager().restoreAuthToken(iterableKeychain, authDataRestoreExecutor); } private class IterableApiAuthProvider implements IterableApiClient.AuthProvider { @@ -698,6 +694,10 @@ void setAuthToken(String authToken, boolean bypassAuth) { } } + void setRestoredAuthToken(@Nullable String authToken) { + _authToken = authToken; + } + protected void registerDeviceToken(final @Nullable String email, final @Nullable String userId, final @Nullable String authToken, final @NonNull String applicationName, final @NonNull String deviceToken, final Map deviceAttributes) { if (deviceToken != null) { if (!checkSDKInitialization() && _userIdUnknown == null) { @@ -1997,4 +1997,4 @@ public void trackEmbeddedSession(@NonNull IterableEmbeddedSession session) { //endregion -} \ No newline at end of file +} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java new file mode 100644 index 000000000..9120b31bf --- /dev/null +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java @@ -0,0 +1,216 @@ +package com.iterable.iterableapi; + +import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +/** + * Retries a stored auth-token read without turning a transient timeout into a missing token. + */ +class IterableAuthDataRestorer { + private static final String TAG = "IterableAuthRestore"; + + @VisibleForTesting + static final int MAX_TIMEOUT_RETRIES = 2; + + @VisibleForTesting + static final long RETRY_DELAY_MS = 1000L; + + private static final ScheduledExecutorService RETRY_EXECUTOR = + Executors.newSingleThreadScheduledExecutor(runnable -> { + Thread thread = new Thread(runnable, "IterableAuthRestore"); + thread.setDaemon(true); + return thread; + }); + + interface Callback { + void onAuthTokenRestored(@Nullable String authToken); + + void onAuthTokenUnavailable(); + } + + private final IterableKeychain keychain; + private final ScheduledExecutorService retryExecutor; + + @Nullable + private ScheduledFuture pendingRetry; + + @Nullable + private Callback callback; + + private int generation; + private boolean restoring; + private boolean unavailable; + + IterableAuthDataRestorer(IterableKeychain keychain) { + this(keychain, RETRY_EXECUTOR); + } + + @VisibleForTesting + IterableAuthDataRestorer( + IterableKeychain keychain, + ScheduledExecutorService retryExecutor) { + this.keychain = keychain; + this.retryExecutor = retryExecutor; + } + + void restore(KeychainReadResult initialRead, Callback callback) { + final int currentGeneration; + synchronized (this) { + cancelPendingRetry(); + generation++; + currentGeneration = generation; + this.callback = callback; + restoring = true; + unavailable = false; + } + + IterableLogger.d(TAG, "auth_restore action=start"); + handleRead(currentGeneration, 0, initialRead); + } + + /** + * Returns true while restoration is unresolved. An unavailable restore starts a new cycle; + * an already-running restore is left alone. + */ + synchronized boolean resumeIfUnresolved() { + if (restoring) { + IterableLogger.d( + TAG, + "auth_restore action=resume outcome=already_running source=foreground"); + return true; + } + if (!unavailable || callback == null) { + return false; + } + + generation++; + restoring = true; + unavailable = false; + IterableLogger.d( + TAG, + "auth_restore action=resume outcome=scheduled source=foreground"); + scheduleRead(generation, 0, 0); + return true; + } + + synchronized void cancel(String reason) { + boolean wasUnresolved = restoring || unavailable; + generation++; + cancelPendingRetry(); + callback = null; + restoring = false; + unavailable = false; + if (wasUnresolved) { + IterableLogger.d(TAG, "auth_restore action=cancel reason=" + reason); + } + } + + private void handleRead( + int currentGeneration, + int timeoutRetries, + KeychainReadResult result) { + if (!isCurrent(currentGeneration)) { + return; + } + + int attempt = timeoutRetries + 1; + if (result instanceof KeychainReadResult.Value) { + String authToken = ((KeychainReadResult.Value) result).getValue(); + complete(currentGeneration, authToken, attempt); + return; + } + + IterableLogger.d( + TAG, + "auth_restore action=read attempt=" + + attempt + + " outcome=timeout"); + if (timeoutRetries >= MAX_TIMEOUT_RETRIES) { + markUnavailable(currentGeneration, attempt); + } else { + scheduleRead(currentGeneration, timeoutRetries + 1, RETRY_DELAY_MS); + } + } + + private synchronized void scheduleRead( + int currentGeneration, + int timeoutRetries, + long delayMs) { + if (!isCurrent(currentGeneration)) { + return; + } + + pendingRetry = retryExecutor.schedule(() -> { + synchronized (IterableAuthDataRestorer.this) { + if (!isCurrent(currentGeneration)) { + IterableLogger.d( + TAG, + "auth_restore action=ignore reason=stale_generation"); + return; + } + pendingRetry = null; + } + handleRead(currentGeneration, timeoutRetries, keychain.readAuthToken()); + }, delayMs, TimeUnit.MILLISECONDS); + } + + private synchronized void complete( + int currentGeneration, + @Nullable String authToken, + int attempt) { + if (!isCurrent(currentGeneration)) { + return; + } + + restoring = false; + unavailable = false; + pendingRetry = null; + IterableLogger.d( + TAG, + "auth_restore action=read attempt=" + + attempt + + " outcome=" + + (authToken == null ? "token_missing" : "token_found")); + + Callback currentCallback = callback; + callback = null; + if (currentCallback != null) { + // Keep completion ordered with cancel(): an explicit identity change must win. + currentCallback.onAuthTokenRestored(authToken); + } + } + + private synchronized void markUnavailable(int currentGeneration, int attempts) { + if (!isCurrent(currentGeneration)) { + return; + } + + restoring = false; + unavailable = true; + pendingRetry = null; + IterableLogger.w( + TAG, + "auth_restore action=complete attempts=" + + attempts + + " outcome=unavailable"); + if (callback != null) { + callback.onAuthTokenUnavailable(); + } + } + + private synchronized boolean isCurrent(int currentGeneration) { + return generation == currentGeneration && restoring; + } + + private void cancelPendingRetry() { + if (pendingRetry != null) { + pendingRetry.cancel(true); + pendingRetry = null; + } + } +} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index c6db12ef7..ac94978f5 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -14,6 +14,7 @@ import java.util.TimerTask; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; public class IterableAuthManager implements IterableActivityMonitor.AppStateCallback { private static final String TAG = "IterableAuth"; @@ -24,11 +25,13 @@ public class IterableAuthManager implements IterableActivityMonitor.AppStateCall * VALID: Last request succeeded with this token. * INVALID: A 401 JWT error was received; processing should pause. * UNKNOWN: A new token was obtained but not yet verified by a request. + * RESTORING: Startup auth is unresolved, so JWT-required work must wait. */ enum AuthState { VALID, INVALID, - UNKNOWN + UNKNOWN, + RESTORING } /** @@ -44,6 +47,12 @@ interface AuthTokenReadyListener { private final IterableActivityMonitor activityMonitor; @VisibleForTesting Timer timer; + @VisibleForTesting + volatile TimerTask scheduledRefreshTask; + @VisibleForTesting + volatile IterableAuthRefreshReason scheduledRefreshReason; + @Nullable + private volatile IterableAuthDataRestorer authDataRestorer; private boolean hasFailedPriorAuth; private boolean pendingAuth; private boolean requiresAuthRefresh; @@ -51,7 +60,6 @@ interface AuthTokenReadyListener { boolean pauseAuthRetry; int retryCount; private boolean isLastAuthTokenValid; - private volatile boolean isTimerScheduled; private volatile boolean isInForeground = true; // Assume foreground initially private volatile AuthState authState = AuthState.UNKNOWN; @@ -86,7 +94,7 @@ boolean isAuthTokenReady() { if (authHandler == null) { return true; } - return authState != AuthState.INVALID; + return isReadyState(authState); } /** @@ -101,19 +109,77 @@ AuthState getAuthState() { } /** - * Centralized auth state setter. Notifies AuthTokenReadyListeners only when - * transitioning from INVALID to a ready state (UNKNOWN or VALID), which means - * a new token has been obtained after a prior auth failure. + * Centralized auth state setter. Listeners are notified whenever auth moves from a blocked + * state to a ready state. */ private void setAuthState(AuthState newState) { AuthState previousState = this.authState; this.authState = newState; - if (previousState == AuthState.INVALID && newState != AuthState.INVALID) { + if (!isReadyState(previousState) && isReadyState(newState)) { notifyAuthTokenReadyListeners(); } } + private boolean isReadyState(AuthState state) { + return state == AuthState.VALID || state == AuthState.UNKNOWN; + } + + @Nullable + String restoreAuthToken( + IterableKeychain keychain, + @Nullable ScheduledExecutorService retryExecutor) { + KeychainReadResult initialRead = keychain.readAuthToken(); + setAuthState(AuthState.RESTORING); + authDataRestorer = retryExecutor == null + ? new IterableAuthDataRestorer(keychain) + : new IterableAuthDataRestorer(keychain, retryExecutor); + authDataRestorer.restore(initialRead, new IterableAuthDataRestorer.Callback() { + @Override + public void onAuthTokenRestored(@Nullable String authToken) { + api.setRestoredAuthToken(authToken); + authDataRestorer = null; + handleRestoredAuthToken(authToken); + } + + @Override + public void onAuthTokenUnavailable() { + IterableLogger.w( + TAG, + "auth_restore action=block reason=storage_unavailable"); + } + }); + return initialRead.valueOrNull(); + } + + private void handleRestoredAuthToken(@Nullable String authToken) { + if (authToken != null) { + setAuthState(AuthState.UNKNOWN); + queueExpirationRefresh(authToken); + } else { + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.STORED_TOKEN_MISSING, + null); + } + } + + void cancelAuthTokenRestore(String reason) { + if (authDataRestorer != null) { + authDataRestorer.cancel(reason); + authDataRestorer = null; + } + if (authState == AuthState.RESTORING) { + setAuthState(AuthState.UNKNOWN); + } + } + + void useExplicitAuthToken(String authToken) { + cancelAuthTokenRestore("explicit_token"); + api.setAuthToken(authToken); + queueExpirationRefresh(authToken); + } + private void notifyAuthTokenReadyListeners() { ArrayList listenersCopy = new ArrayList<>(authTokenReadyListeners); for (AuthTokenReadyListener listener : listenersCopy) { @@ -131,10 +197,15 @@ public void pauseAuthRetries(boolean pauseRetry) { } void reset() { - clearRefreshTimer(); + clearRefreshTimer("auth_reset"); setIsLastAuthTokenValid(false); } + void resetForIdentityChange() { + cancelAuthTokenRestore("identity_changed"); + reset(); + } + void setIsLastAuthTokenValid(boolean isValid) { isLastAuthTokenValid = isValid; if (isValid) { @@ -223,7 +294,10 @@ private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHand } else { handleAuthFailure(authToken, AuthFailureReason.AUTH_TOKEN_NULL); IterableApi.getInstance().setAuthToken(authToken); - scheduleAuthTokenRefresh(getNextRetryInterval(), false, null); + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.AUTH_HANDLER_RETRY, + null); return; } reSyncAuth(); @@ -235,32 +309,45 @@ private void handleAuthTokenFailure(Throwable throwable) { IterableLogger.e(TAG, "Error while requesting Auth Token", throwable); handleAuthFailure(null, AuthFailureReason.AUTH_TOKEN_GENERATION_ERROR); pendingAuth = false; - scheduleAuthTokenRefresh(getNextRetryInterval(), false, null); + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.AUTH_HANDLER_RETRY, + null); } public void queueExpirationRefresh(@Nullable String encodedJWT) { - clearRefreshTimer(); + clearRefreshTimer("token_replaced"); try { if (encodedJWT == null) { IterableLogger.d(TAG, "JWT is null. Scheduling token refresh"); - if (!isTimerScheduled) { - scheduleAuthTokenRefresh(getNextRetryInterval(), false, null); - } + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.TOKEN_MISSING, + null); return; } long expirationTimeSeconds = decodedExpiration(encodedJWT); long triggerExpirationRefreshTime = expirationTimeSeconds * 1000L - expiringAuthTokenRefreshPeriod - IterableUtil.currentTimeMillis(); if (triggerExpirationRefreshTime > 0) { - scheduleAuthTokenRefresh(triggerExpirationRefreshTime, true, null); + scheduleAuthTokenRefresh( + triggerExpirationRefreshTime, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null); } else { - scheduleAuthTokenRefresh(getNextRetryInterval(), true, null); + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.TOKEN_EXPIRED, + null); } } catch (Exception e) { IterableLogger.e(TAG, "Error while parsing JWT for the expiration", e); isLastAuthTokenValid = false; handleAuthFailure(encodedJWT, AuthFailureReason.AUTH_TOKEN_PAYLOAD_INVALID); - scheduleAuthTokenRefresh(getNextRetryInterval(), false, null); + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.TOKEN_INVALID, + null); } } @@ -271,7 +358,10 @@ void resetFailedAuth() { void reSyncAuth() { if (requiresAuthRefresh) { requiresAuthRefresh = false; - scheduleAuthTokenRefresh(getNextRetryInterval(), false, null); + scheduleAuthTokenRefresh( + getNextRetryInterval(), + IterableAuthRefreshReason.DEFERRED_REFRESH, + null); } } @@ -292,35 +382,100 @@ long getNextRetryInterval() { return nextRetryInterval; } - 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 + synchronized void scheduleAuthTokenRefresh( + long timeDuration, + IterableAuthRefreshReason reason, + final IterableHelper.SuccessHandler successCallback) { + if (pauseAuthRetry && !reason.ignoresRetryPolicy()) { + IterableLogger.d( + TAG, + "auth_refresh action=skip reason=" + + reason + + " cause=retry_paused"); + return; + } + if (scheduledRefreshTask != null) { + IterableLogger.d( + TAG, + "auth_refresh action=skip reason=" + + reason + + " cause=already_scheduled pending_reason=" + + scheduledRefreshReason); return; } if (timer == null) { timer = new Timer(true); } - 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() { - if (api.getEmail() != null || api.getUserId() != null) { - api.getAuthManager().requestNewAuthToken(false, successCallback, isScheduledRefresh); - } else { - IterableLogger.w(TAG, "Email or userId is not available. Skipping token refresh"); - } - synchronized (IterableAuthManager.this) { - isTimerScheduled = false; - } + final TimerTask refreshTask = new TimerTask() { + @Override + public void run() { + if (!claimRefreshTask(this, reason)) { + return; } - }, timeDuration); + + IterableLogger.d(TAG, "auth_refresh action=fire reason=" + reason); + if (api.getEmail() != null || api.getUserId() != null) { + requestNewAuthToken( + false, + successCallback, + reason.ignoresRetryPolicy()); + } else { + IterableLogger.w( + TAG, + "auth_refresh action=skip reason=" + + reason + + " cause=identity_missing"); + } + } + }; + + try { + scheduledRefreshTask = refreshTask; + scheduledRefreshReason = reason; + timer.schedule(refreshTask, timeDuration); + IterableLogger.d( + TAG, + "auth_refresh action=schedule reason=" + + reason + + " delay_ms=" + + timeDuration); } catch (Exception e) { - isTimerScheduled = false; - IterableLogger.e(TAG, "timer exception: " + timer, e); + releaseRefreshTask(refreshTask); + if (timer != null) { + timer.cancel(); + timer = null; + } + IterableLogger.e( + TAG, + "auth_refresh action=error reason=" + + reason + + " cause=schedule_failed", + e); + } + } + + private synchronized boolean claimRefreshTask( + TimerTask task, + IterableAuthRefreshReason reason) { + if (scheduledRefreshTask != task) { + IterableLogger.d( + TAG, + "auth_refresh action=ignore reason=" + + reason + + " cause=stale_task"); + return false; + } + + scheduledRefreshTask = null; + scheduledRefreshReason = null; + return true; + } + + private synchronized void releaseRefreshTask(TimerTask task) { + if (scheduledRefreshTask == task) { + scheduledRefreshTask = null; + scheduledRefreshReason = null; } } @@ -368,11 +523,28 @@ private void checkAndHandleAuthRefresh() { } } - synchronized void clearRefreshTimer() { + void clearRefreshTimer() { + clearRefreshTimer("explicit_clear"); + } + + private synchronized void clearRefreshTimer(String reason) { + IterableAuthRefreshReason cancelledReason = scheduledRefreshReason; + if (scheduledRefreshTask != null) { + scheduledRefreshTask.cancel(); + } if (timer != null) { timer.cancel(); timer = null; - isTimerScheduled = false; + } + scheduledRefreshTask = null; + scheduledRefreshReason = null; + if (cancelledReason != null) { + IterableLogger.d( + TAG, + "auth_refresh action=cancel reason=" + + cancelledReason + + " cause=" + + reason); } } @@ -381,6 +553,9 @@ public void onSwitchToForeground() { try { IterableLogger.d(TAG, "App switched to foreground - enabling auth token requests"); isInForeground = true; + if (authDataRestorer != null && authDataRestorer.resumeIfUnresolved()) { + return; + } checkAndHandleAuthRefresh(); } catch (Exception e) { IterableLogger.e(TAG, "Error occurred in handling auth token refresh", e); @@ -392,10 +567,9 @@ public void onSwitchToBackground() { try { IterableLogger.d(TAG, "App switched to background - disabling auth token requests"); isInForeground = false; - clearRefreshTimer(); + clearRefreshTimer("app_backgrounded"); } catch (Exception e) { IterableLogger.e(TAG, "Error while switching to background", e); } } } - diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRefreshReason.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRefreshReason.java new file mode 100644 index 000000000..91d20f9fa --- /dev/null +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRefreshReason.java @@ -0,0 +1,28 @@ +package com.iterable.iterableapi; + +/** + * Explains why the SDK scheduled an auth token refresh. + * + * The reason also defines whether the refresh is part of the normal token lifecycle or a retry + * that must respect the configured retry pause and maximum. + */ +enum IterableAuthRefreshReason { + TOKEN_EXPIRING(true), + TOKEN_EXPIRED(true), + STORED_TOKEN_MISSING(true), + TOKEN_MISSING(false), + TOKEN_INVALID(false), + AUTH_HANDLER_RETRY(false), + DEFERRED_REFRESH(false), + JWT_401(false); + + private final boolean ignoresRetryPolicy; + + IterableAuthRefreshReason(boolean ignoresRetryPolicy) { + this.ignoresRetryPolicy = ignoresRetryPolicy; + } + + boolean ignoresRetryPolicy() { + return ignoresRetryPolicy; + } +} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt index 1d5b22ba0..67f58156e 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt @@ -2,11 +2,20 @@ package com.iterable.iterableapi import android.content.Context import android.content.SharedPreferences +import androidx.annotation.RestrictTo import java.util.concurrent.Callable import java.util.concurrent.Executors import java.util.concurrent.TimeoutException import java.util.concurrent.TimeUnit +@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) +sealed interface KeychainReadResult { + data class Value(val value: String?) : KeychainReadResult + data object TimedOut : KeychainReadResult + + fun valueOrNull(): String? = (this as? Value)?.value +} + class IterableKeychain { companion object { private const val TAG = "IterableKeychain" @@ -114,31 +123,35 @@ class IterableKeychain { } } - private fun secureGet(key: String): String? { + private fun secureGet(key: String): String? = + when (val result = readValue(key)) { + is KeychainReadResult.Value -> result.value + KeychainReadResult.TimedOut -> null + } + + private fun readValue(key: String): KeychainReadResult { val hasPlainText = sharedPrefs.getBoolean(key + PLAINTEXT_SUFFIX, false) if (!encryption) { - if (hasPlainText) { - return sharedPrefs.getString(key, null) - } else { - return null - } + val value = if (hasPlainText) sharedPrefs.getString(key, null) else null + return KeychainReadResult.Value(value) } else if (hasPlainText) { - return sharedPrefs.getString(key, null) + return KeychainReadResult.Value(sharedPrefs.getString(key, null)) } - - val encryptedValue = sharedPrefs.getString(key, null) ?: return null + + val encryptedValue = sharedPrefs.getString(key, null) + ?: return KeychainReadResult.Value(null) return try { - encryptor?.let { runWithTimeout { it.decrypt(encryptedValue) } } + KeychainReadResult.Value(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) + // force a re-login (and a new auth-token request) on every slow launch. Keep the timeout + // distinct so auth restoration cannot mistake it for a missing value. (SDK-547) IterableLogger.w(TAG, "Crypto operation timed out; keeping encrypted data for retry.") - null + KeychainReadResult.TimedOut } catch (e: Exception) { handleDecryptionError(e) - null + KeychainReadResult.Value(null) } } @@ -184,6 +197,8 @@ class IterableKeychain { fun saveUserId(userId: String?) = secureSave(KEY_USER_ID, userId) fun getAuthToken() = secureGet(KEY_AUTH_TOKEN) + @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) + fun readAuthToken() = readValue(KEY_AUTH_TOKEN) fun saveAuthToken(authToken: String?) = secureSave(KEY_AUTH_TOKEN, authToken) fun getUserIdUnknown() = secureGet(KEY_UNKNOWN_USER_ID) diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java index 33ecddd3f..45728106d 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java @@ -273,7 +273,10 @@ private static void handleJwtAuthRetry(IterableApiRequest iterableApiRequest) { IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); authManager.setIsLastAuthTokenValid(false); long retryInterval = authManager.getNextRetryInterval(); - authManager.scheduleAuthTokenRefresh(retryInterval, false, null); + authManager.scheduleAuthTokenRefresh( + retryInterval, + IterableAuthRefreshReason.JWT_401, + null); } else { requestNewAuthTokenAndRetry(iterableApiRequest); } @@ -427,7 +430,10 @@ private void handleErrorResponse(IterableApiResponse response) { private static void requestNewAuthTokenAndRetry(IterableApiRequest iterableApiRequest) { IterableApi.getInstance().getAuthManager().setIsLastAuthTokenValid(false); long retryInterval = IterableApi.getInstance().getAuthManager().getNextRetryInterval(); - IterableApi.getInstance().getAuthManager().scheduleAuthTokenRefresh(retryInterval, false, data -> { + IterableApi.getInstance().getAuthManager().scheduleAuthTokenRefresh( + retryInterval, + IterableAuthRefreshReason.JWT_401, + data -> { try { String newAuthToken = data.getString("newAuthToken"); retryRequestWithNewAuthToken(newAuthToken, iterableApiRequest); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java index be1361079..b2ee90989 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java @@ -541,11 +541,9 @@ public void testForegroundWithValidTokenDoesNotRequestNewToken() throws Exceptio 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. + // SDK-547: concurrent callers (foreground refresh, 401 retry, an already-firing timer) must all + // contend for one scheduler-owned task. The pending task, rather than a separate boolean, is the + // source of truth. // // We drive scheduleAuthTokenRefresh directly rather than requestNewAuthToken: the executor is // not injectable (see @Ignore'd tests above) and its pendingAuth guard serializes calls, @@ -556,8 +554,7 @@ public void testConcurrentScheduleAuthTokenRefreshSchedulesOnlyOneTimer() throws 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. + // Fake timer that counts schedule() calls and holds briefly to maximize contention. Timer countingTimer = new Timer(true) { @Override public void schedule(TimerTask task, long delay) { @@ -578,7 +575,10 @@ public void schedule(TimerTask task, long delay) { threads[i] = new Thread(() -> { try { barrier.await(); - authManager.scheduleAuthTokenRefresh(60000, true, null); + authManager.scheduleAuthTokenRefresh( + 60000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java new file mode 100644 index 000000000..6cf7743dc --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java @@ -0,0 +1,165 @@ +package com.iterable.iterableapi; + +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayDeque; +import java.util.Queue; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +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.mockito.Mockito.when; + +public class IterableAuthDataRestoreIntegrationTest extends BaseTest { + private static final String EMAIL = "user@example.com"; + private static final String NEW_EMAIL = "new@example.com"; + private static final String VALID_JWT = + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9." + + "mYtgSqdUIxK8_RnYBTUP4cmpKw83aKi7cMiixF3qMB4"; + + private IterableAuthHandler authHandler; + private IterableKeychain keychain; + private ScheduledExecutorService retryExecutor; + private Queue scheduledTasks; + + @Before + public void setUp() { + IterableApi.sharedInstance = new IterableApi(); + authHandler = mock(IterableAuthHandler.class); + keychain = mock(IterableKeychain.class); + doReturn(EMAIL).when(keychain).getEmail(); + + retryExecutor = mock(ScheduledExecutorService.class); + scheduledTasks = new ArrayDeque<>(); + when(retryExecutor.schedule( + any(Runnable.class), + anyLong(), + eq(TimeUnit.MILLISECONDS))) + .thenAnswer(invocation -> { + scheduledTasks.add(invocation.getArgument(0)); + return mock(ScheduledFuture.class); + }); + + IterableApi.sharedInstance.keychain = keychain; + IterableApi.sharedInstance.authDataRestoreExecutor = retryExecutor; + } + + @Test + public void repeatedTimeoutsNeverRequestANewTokenWithoutConfirmedAbsence() { + doReturn(KeychainReadResult.TimedOut.INSTANCE) + .when(keychain) + .readAuthToken(); + + initialize(); + runNext(); + runNext(); + + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + assertEquals(IterableAuthManager.AuthState.RESTORING, authManager.getAuthState()); + assertFalse(authManager.isAuthTokenReady()); + verify(authHandler, never()).onAuthTokenRequested(); + + authManager.onSwitchToForeground(); + + assertEquals(1, scheduledTasks.size()); + verify(authHandler, never()).onAuthTokenRequested(); + } + + @Test + public void timeoutThenSuccessRestoresTokenWithoutCallingClientHandler() { + doReturn( + KeychainReadResult.TimedOut.INSTANCE, + new KeychainReadResult.Value(VALID_JWT)) + .when(keychain) + .readAuthToken(); + + initialize(); + runNext(); + + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + assertEquals(EMAIL, IterableApi.getInstance().getEmail()); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + assertTrue(authManager.isAuthTokenReady()); + assertEquals( + IterableAuthRefreshReason.TOKEN_EXPIRING, + authManager.scheduledRefreshReason); + verify(authHandler, never()).onAuthTokenRequested(); + } + + @Test + public void confirmedMissingTokenSchedulesOneReasonedRefresh() { + doReturn(new KeychainReadResult.Value(null)) + .when(keychain) + .readAuthToken(); + + initialize(); + + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + assertEquals(IterableAuthManager.AuthState.RESTORING, authManager.getAuthState()); + assertFalse(authManager.isAuthTokenReady()); + assertEquals( + IterableAuthRefreshReason.STORED_TOKEN_MISSING, + authManager.scheduledRefreshReason); + verify(authHandler, never()).onAuthTokenRequested(); + } + + @Test + public void explicitTokenAfterConfirmedAbsenceUnblocksAuth() { + doReturn(new KeychainReadResult.Value(null)) + .when(keychain) + .readAuthToken(); + + initialize(); + IterableApi.getInstance().setEmail(EMAIL, VALID_JWT); + + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + assertTrue(authManager.isAuthTokenReady()); + assertEquals( + IterableAuthRefreshReason.TOKEN_EXPIRING, + authManager.scheduledRefreshReason); + verify(authHandler, never()).onAuthTokenRequested(); + } + + @Test + public void explicitLoginWinsOverAQueuedRestoreRetry() { + doReturn( + KeychainReadResult.TimedOut.INSTANCE, + new KeychainReadResult.Value("old-token")) + .when(keychain) + .readAuthToken(); + + initialize(); + IterableApi.getInstance().setEmail(NEW_EMAIL, VALID_JWT); + runNext(); + + assertEquals(NEW_EMAIL, IterableApi.getInstance().getEmail()); + assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + } + + private void initialize() { + IterableApi.initialize( + getContext(), + "apiKey", + new IterableConfig.Builder() + .setAutoPushRegistration(false) + .setAuthHandler(authHandler) + .build()); + } + + private void runNext() { + scheduledTasks.remove().run(); + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java new file mode 100644 index 000000000..7e117a706 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java @@ -0,0 +1,130 @@ +package com.iterable.iterableapi; + +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayDeque; +import java.util.Queue; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class IterableAuthDataRestorerTest { + private IterableKeychain keychain; + private ScheduledExecutorService retryExecutor; + private Queue scheduledTasks; + private RecordingCallback callback; + private IterableAuthDataRestorer restorer; + + @Before + public void setUp() { + keychain = mock(IterableKeychain.class); + retryExecutor = mock(ScheduledExecutorService.class); + scheduledTasks = new ArrayDeque<>(); + when(retryExecutor.schedule( + any(Runnable.class), + anyLong(), + eq(TimeUnit.MILLISECONDS))) + .thenAnswer(invocation -> { + scheduledTasks.add(invocation.getArgument(0)); + return mock(ScheduledFuture.class); + }); + + callback = new RecordingCallback(); + restorer = new IterableAuthDataRestorer(keychain, retryExecutor); + } + + @Test + public void timeoutThenSuccessfulReadRestoresToken() { + doReturn(new KeychainReadResult.Value("stored-token")) + .when(keychain) + .readAuthToken(); + + restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); + runNext(); + + assertEquals("stored-token", callback.restoredToken); + assertEquals(0, callback.unavailableCount); + } + + @Test + public void exhaustingTimeoutRetriesRemainsUnavailable() { + doReturn(KeychainReadResult.TimedOut.INSTANCE) + .when(keychain) + .readAuthToken(); + + restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); + runNext(); + runNext(); + + verify(keychain, times(IterableAuthDataRestorer.MAX_TIMEOUT_RETRIES)) + .readAuthToken(); + assertNull(callback.restoredToken); + assertEquals(1, callback.unavailableCount); + assertTrue(restorer.resumeIfUnresolved()); + assertEquals(1, scheduledTasks.size()); + } + + @Test + public void foregroundRetryCanRecoverAfterAnUnavailableCycle() { + doReturn( + KeychainReadResult.TimedOut.INSTANCE, + KeychainReadResult.TimedOut.INSTANCE, + new KeychainReadResult.Value("stored-token")) + .when(keychain) + .readAuthToken(); + + restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); + runNext(); + runNext(); + assertEquals(1, callback.unavailableCount); + + assertTrue(restorer.resumeIfUnresolved()); + runNext(); + + assertEquals("stored-token", callback.restoredToken); + } + + @Test + public void cancellationPreventsAQueuedRetryFromRestoringAStaleToken() { + restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); + restorer.cancel("identity_changed"); + runNext(); + + assertNull(callback.restoredToken); + assertEquals(0, callback.unavailableCount); + verify(keychain, never()).readAuthToken(); + } + + private void runNext() { + scheduledTasks.remove().run(); + } + + private static class RecordingCallback implements IterableAuthDataRestorer.Callback { + String restoredToken; + int unavailableCount; + + @Override + public void onAuthTokenRestored(String authToken) { + restoredToken = authToken; + } + + @Override + public void onAuthTokenUnavailable() { + unavailableCount++; + } + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java new file mode 100644 index 000000000..af70ab0f6 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java @@ -0,0 +1,127 @@ +package com.iterable.iterableapi; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.ExecutorService; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class IterableAuthRefreshOwnershipTest extends BaseTest { + private IterableApi api; + private IterableAuthManager authManager; + private ExecutorService executor; + + @Before + public void setUp() { + api = mock(IterableApi.class); + when(api.getEmail()).thenReturn("user@example.com"); + + authManager = new IterableAuthManager( + api, + mock(IterableAuthHandler.class), + new RetryPolicy(3, 1, RetryPolicy.Type.LINEAR), + 60_000); + executor = mock(ExecutorService.class); + authManager.executor = executor; + } + + @After + public void tearDown() { + authManager.clearRefreshTimer(); + } + + @Test + public void staleTaskCannotRunOrClearItsReplacement() { + RetainingTimer firstTimer = new RetainingTimer(); + authManager.timer = firstTimer; + authManager.scheduleAuthTokenRefresh( + 1000, + IterableAuthRefreshReason.AUTH_HANDLER_RETRY, + null); + TimerTask staleTask = firstTimer.lastTask(); + + authManager.clearRefreshTimer(); + RetainingTimer replacementTimer = new RetainingTimer(); + authManager.timer = replacementTimer; + authManager.scheduleAuthTokenRefresh( + 2000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null); + TimerTask replacementTask = replacementTimer.lastTask(); + + staleTask.run(); + + assertSame(replacementTask, authManager.scheduledRefreshTask); + assertEquals( + IterableAuthRefreshReason.TOKEN_EXPIRING, + authManager.scheduledRefreshReason); + verify(executor, never()).submit(any(Runnable.class)); + + replacementTask.run(); + + assertNull(authManager.scheduledRefreshTask); + assertNull(authManager.scheduledRefreshReason); + verify(executor).submit(any(Runnable.class)); + } + + @Test + public void firingTaskReleasesOwnershipBeforeAnotherRefreshIsScheduled() { + RetainingTimer timer = new RetainingTimer(); + authManager.timer = timer; + authManager.scheduleAuthTokenRefresh( + 1000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null); + + timer.lastTask().run(); + assertNull(authManager.scheduledRefreshTask); + + authManager.scheduleAuthTokenRefresh( + 2000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null); + + assertEquals(2, timer.taskCount()); + assertSame(timer.lastTask(), authManager.scheduledRefreshTask); + } + + private static class RetainingTimer extends Timer { + private final List tasks = new ArrayList<>(); + + RetainingTimer() { + super(true); + super.cancel(); + } + + @Override + public void schedule(TimerTask task, long delay) { + tasks.add(task); + } + + @Override + public void cancel() { + // Retain tasks so a test can run a task after cancellation. + } + + TimerTask lastTask() { + return tasks.get(tasks.size() - 1); + } + + int taskCount() { + return tasks.size(); + } + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java index fb04c2951..980daa669 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java @@ -233,6 +233,9 @@ public void backgroundCancelsTheScheduledRefresh() { authManager.onSwitchToBackground(); assertNull("backgrounding must cancel the refresh timer", authManager.timer); + assertNull( + "backgrounding must release refresh ownership", + authManager.scheduledRefreshTask); } @Test @@ -269,7 +272,12 @@ public void tokenRequestIsSkippedWhileBackgrounded() { public void concurrentSchedulingArmsOnlyOneRefresh() throws Exception { CountingTimer timer = installCountingTimer(); - runConcurrently(8, () -> authManager.scheduleAuthTokenRefresh(60_000, true, null)); + runConcurrently( + 8, + () -> authManager.scheduleAuthTokenRefresh( + 60_000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null)); assertEquals(1, timer.liveTaskCount()); } @@ -344,7 +352,7 @@ private void settle() { } private boolean isRefreshScheduled() { - return authManager.timer != null; + return authManager.scheduledRefreshTask != null; } private CountingTimer installCountingTimer() { @@ -357,7 +365,10 @@ private CountingTimer installCountingTimer() { private CountingTimer armObservableRefresh() { authManager.clearRefreshTimer(); CountingTimer timer = installCountingTimer(); - authManager.scheduleAuthTokenRefresh(60_000, true, null); + authManager.scheduleAuthTokenRefresh( + 60_000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null); assertEquals(1, timer.liveTaskCount()); return timer; } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt index 3d0ccc913..41039aace 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt @@ -202,6 +202,30 @@ class IterableKeychainTest { verify(mockDecryptionFailureHandler, never()).onDecryptionFailed(any()) } + @Test + fun testReadAuthTokenDistinguishesAStoredValue() { + `when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())) + .thenReturn("encrypted_stored-token") + + val result = keychain.readAuthToken() + + assertEquals(KeychainReadResult.Value("stored-token"), result) + } + + @Test + fun testReadAuthTokenDistinguishesATimeoutFromAMissingValue() { + `when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())) + .thenReturn("slow_token") + `when`(mockEncryptor.decrypt(eq("slow_token"))).thenAnswer { + Thread.sleep(700) + "stored-token" + } + + val result = keychain.readAuthToken() + + assertEquals(KeychainReadResult.TimedOut, result) + } + @Test fun testCryptoTimeoutDoesNotBlockSubsequentReads() { // SDK-547: crypto runs on a single-thread executor. A slow op that times out must be @@ -473,4 +497,4 @@ class IterableKeychainTest { // - Existing encrypted data that can't be decrypted returns null (graceful degradation) // - No crashes occur during the failure scenario } -} \ No newline at end of file +} From 86b5d9697dc5cce38345877256411a74976b8688 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Tue, 18 Aug 2026 11:41:45 +0100 Subject: [PATCH 06/11] [SDK-547] Clarify auth lifecycle ownership --- .../com/iterable/iterableapi/IterableApi.java | 2 +- .../iterableapi/IterableAuthDataRestorer.java | 68 +++++---- .../iterableapi/IterableAuthManager.java | 131 +++++++++++------- .../iterableapi/IterableRequestTask.java | 18 +-- .../iterableapi/IterableApiAuthTests.java | 3 +- ...terableAuthDataRestoreIntegrationTest.java | 42 ++++-- .../IterableAuthDataRestorerTest.java | 19 ++- .../IterableAuthRefreshOwnershipTest.java | 46 +++++- .../IterableAuthTokenLifecycleTest.java | 10 +- 9 files changed, 225 insertions(+), 114 deletions(-) diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index a24985088..c4d43a546 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -433,7 +433,7 @@ private void onLogin( getAuthManager().pauseAuthRetries(false); if (authToken != null) { - setAuthToken(authToken); + getAuthManager().useExplicitAuthToken(authToken); attemptMergeAndEventReplay(userIdOrEmail, isEmail, merge, replay, isUnknown, failureHandler); } else { getAuthManager().requestNewAuthToken(false, data -> attemptMergeAndEventReplay(userIdOrEmail, isEmail, merge, replay, isUnknown, failureHandler)); diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java index 9120b31bf..af59369c9 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java @@ -21,11 +21,13 @@ class IterableAuthDataRestorer { static final long RETRY_DELAY_MS = 1000L; private static final ScheduledExecutorService RETRY_EXECUTOR = - Executors.newSingleThreadScheduledExecutor(runnable -> { - Thread thread = new Thread(runnable, "IterableAuthRestore"); - thread.setDaemon(true); - return thread; - }); + Executors.newSingleThreadScheduledExecutor( + runnable -> { + Thread thread = new Thread(runnable, "IterableAuthRestore"); + thread.setDaemon(true); + return thread; + } + ); interface Callback { void onAuthTokenRestored(@Nullable String authToken); @@ -53,7 +55,8 @@ interface Callback { @VisibleForTesting IterableAuthDataRestorer( IterableKeychain keychain, - ScheduledExecutorService retryExecutor) { + ScheduledExecutorService retryExecutor + ) { this.keychain = keychain; this.retryExecutor = retryExecutor; } @@ -81,7 +84,8 @@ synchronized boolean resumeIfUnresolved() { if (restoring) { IterableLogger.d( TAG, - "auth_restore action=resume outcome=already_running source=foreground"); + "auth_restore action=resume outcome=already_running source=foreground" + ); return true; } if (!unavailable || callback == null) { @@ -93,7 +97,8 @@ synchronized boolean resumeIfUnresolved() { unavailable = false; IterableLogger.d( TAG, - "auth_restore action=resume outcome=scheduled source=foreground"); + "auth_restore action=resume outcome=scheduled source=foreground" + ); scheduleRead(generation, 0, 0); return true; } @@ -113,7 +118,8 @@ synchronized void cancel(String reason) { private void handleRead( int currentGeneration, int timeoutRetries, - KeychainReadResult result) { + KeychainReadResult result + ) { if (!isCurrent(currentGeneration)) { return; } @@ -129,7 +135,8 @@ private void handleRead( TAG, "auth_restore action=read attempt=" + attempt - + " outcome=timeout"); + + " outcome=timeout" + ); if (timeoutRetries >= MAX_TIMEOUT_RETRIES) { markUnavailable(currentGeneration, attempt); } else { @@ -140,29 +147,36 @@ private void handleRead( private synchronized void scheduleRead( int currentGeneration, int timeoutRetries, - long delayMs) { + long delayMs + ) { if (!isCurrent(currentGeneration)) { return; } - pendingRetry = retryExecutor.schedule(() -> { - synchronized (IterableAuthDataRestorer.this) { - if (!isCurrent(currentGeneration)) { - IterableLogger.d( - TAG, - "auth_restore action=ignore reason=stale_generation"); - return; - } - pendingRetry = null; - } - handleRead(currentGeneration, timeoutRetries, keychain.readAuthToken()); - }, delayMs, TimeUnit.MILLISECONDS); + pendingRetry = retryExecutor.schedule( + () -> { + synchronized (IterableAuthDataRestorer.this) { + if (!isCurrent(currentGeneration)) { + IterableLogger.d( + TAG, + "auth_restore action=ignore reason=stale_generation" + ); + return; + } + pendingRetry = null; + } + handleRead(currentGeneration, timeoutRetries, keychain.readAuthToken()); + }, + delayMs, + TimeUnit.MILLISECONDS + ); } private synchronized void complete( int currentGeneration, @Nullable String authToken, - int attempt) { + int attempt + ) { if (!isCurrent(currentGeneration)) { return; } @@ -175,7 +189,8 @@ private synchronized void complete( "auth_restore action=read attempt=" + attempt + " outcome=" - + (authToken == null ? "token_missing" : "token_found")); + + (authToken == null ? "token_missing" : "token_found") + ); Callback currentCallback = callback; callback = null; @@ -197,7 +212,8 @@ private synchronized void markUnavailable(int currentGeneration, int attempts) { TAG, "auth_restore action=complete attempts=" + attempts - + " outcome=unavailable"); + + " outcome=unavailable" + ); if (callback != null) { callback.onAuthTokenUnavailable(); } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index ac94978f5..4537c0103 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -34,6 +34,13 @@ enum AuthState { RESTORING } + private enum RefreshCancellationReason { + AUTH_RESET, + TOKEN_REPLACED, + EXPLICIT_CLEAR, + APP_BACKGROUNDED + } + /** * Listener interface for components that need to react when a new auth token is ready. */ @@ -128,27 +135,32 @@ private boolean isReadyState(AuthState state) { @Nullable String restoreAuthToken( IterableKeychain keychain, - @Nullable ScheduledExecutorService retryExecutor) { + @Nullable ScheduledExecutorService retryExecutor + ) { KeychainReadResult initialRead = keychain.readAuthToken(); setAuthState(AuthState.RESTORING); authDataRestorer = retryExecutor == null ? new IterableAuthDataRestorer(keychain) : new IterableAuthDataRestorer(keychain, retryExecutor); - authDataRestorer.restore(initialRead, new IterableAuthDataRestorer.Callback() { - @Override - public void onAuthTokenRestored(@Nullable String authToken) { - api.setRestoredAuthToken(authToken); - authDataRestorer = null; - handleRestoredAuthToken(authToken); - } - - @Override - public void onAuthTokenUnavailable() { - IterableLogger.w( - TAG, - "auth_restore action=block reason=storage_unavailable"); - } - }); + authDataRestorer.restore( + initialRead, + new IterableAuthDataRestorer.Callback() { + @Override + public void onAuthTokenRestored(@Nullable String authToken) { + api.setRestoredAuthToken(authToken); + authDataRestorer = null; + handleRestoredAuthToken(authToken); + } + + @Override + public void onAuthTokenUnavailable() { + IterableLogger.w( + TAG, + "auth_restore action=block reason=storage_unavailable" + ); + } + } + ); return initialRead.valueOrNull(); } @@ -160,7 +172,8 @@ private void handleRestoredAuthToken(@Nullable String authToken) { scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.STORED_TOKEN_MISSING, - null); + null + ); } } @@ -169,15 +182,15 @@ void cancelAuthTokenRestore(String reason) { authDataRestorer.cancel(reason); authDataRestorer = null; } - if (authState == AuthState.RESTORING) { - setAuthState(AuthState.UNKNOWN); - } } void useExplicitAuthToken(String authToken) { cancelAuthTokenRestore("explicit_token"); api.setAuthToken(authToken); - queueExpirationRefresh(authToken); + if (authHandler != null) { + setAuthState(AuthState.UNKNOWN); + queueExpirationRefresh(authToken); + } } private void notifyAuthTokenReadyListeners() { @@ -197,12 +210,15 @@ public void pauseAuthRetries(boolean pauseRetry) { } void reset() { - clearRefreshTimer("auth_reset"); + clearRefreshTimer(RefreshCancellationReason.AUTH_RESET); setIsLastAuthTokenValid(false); } void resetForIdentityChange() { cancelAuthTokenRestore("identity_changed"); + if (authHandler != null) { + setAuthState(AuthState.RESTORING); + } reset(); } @@ -247,7 +263,7 @@ public void run() { try { if (isLastAuthTokenValid && !shouldIgnoreRetryPolicy) { // if some JWT retry had valid token it will not fetch the auth token again from developer function - handleAuthTokenSuccess(IterableApi.getInstance().getAuthToken(), successCallback); + handleAuthTokenSuccess(api.getAuthToken(), successCallback); pendingAuth = false; return; } @@ -276,16 +292,16 @@ public void run() { } } else { - IterableApi.getInstance().setAuthToken(null, true); + api.setAuthToken(null, true); } } private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHandler successCallback) { if (authToken != null) { - // Token obtained but not yet verified by a request - set state to UNKNOWN. - // setAuthState will notify listeners only if previous state was INVALID. + // Token obtained but not yet verified by a request. Storing it before changing state + // ensures listeners cannot resume JWT work with the previous token. + api.setAuthToken(authToken); setAuthState(AuthState.UNKNOWN); - IterableApi.getInstance().setAuthToken(authToken); queueExpirationRefresh(authToken); if (successCallback != null) { @@ -293,11 +309,12 @@ private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHand } } else { handleAuthFailure(authToken, AuthFailureReason.AUTH_TOKEN_NULL); - IterableApi.getInstance().setAuthToken(authToken); + api.setAuthToken(authToken); scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.AUTH_HANDLER_RETRY, - null); + null + ); return; } reSyncAuth(); @@ -312,18 +329,20 @@ private void handleAuthTokenFailure(Throwable throwable) { scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.AUTH_HANDLER_RETRY, - null); + null + ); } public void queueExpirationRefresh(@Nullable String encodedJWT) { - clearRefreshTimer("token_replaced"); + clearRefreshTimer(RefreshCancellationReason.TOKEN_REPLACED); try { if (encodedJWT == null) { IterableLogger.d(TAG, "JWT is null. Scheduling token refresh"); scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.TOKEN_MISSING, - null); + null + ); return; } @@ -333,12 +352,14 @@ public void queueExpirationRefresh(@Nullable String encodedJWT) { scheduleAuthTokenRefresh( triggerExpirationRefreshTime, IterableAuthRefreshReason.TOKEN_EXPIRING, - null); + null + ); } else { scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.TOKEN_EXPIRED, - null); + null + ); } } catch (Exception e) { IterableLogger.e(TAG, "Error while parsing JWT for the expiration", e); @@ -347,7 +368,8 @@ public void queueExpirationRefresh(@Nullable String encodedJWT) { scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.TOKEN_INVALID, - null); + null + ); } } @@ -361,7 +383,8 @@ void reSyncAuth() { scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.DEFERRED_REFRESH, - null); + null + ); } } @@ -385,13 +408,15 @@ long getNextRetryInterval() { synchronized void scheduleAuthTokenRefresh( long timeDuration, IterableAuthRefreshReason reason, - final IterableHelper.SuccessHandler successCallback) { + final IterableHelper.SuccessHandler successCallback + ) { if (pauseAuthRetry && !reason.ignoresRetryPolicy()) { IterableLogger.d( TAG, "auth_refresh action=skip reason=" + reason - + " cause=retry_paused"); + + " cause=retry_paused" + ); return; } if (scheduledRefreshTask != null) { @@ -400,7 +425,8 @@ synchronized void scheduleAuthTokenRefresh( "auth_refresh action=skip reason=" + reason + " cause=already_scheduled pending_reason=" - + scheduledRefreshReason); + + scheduledRefreshReason + ); return; } if (timer == null) { @@ -419,13 +445,15 @@ public void run() { requestNewAuthToken( false, successCallback, - reason.ignoresRetryPolicy()); + reason.ignoresRetryPolicy() + ); } else { IterableLogger.w( TAG, "auth_refresh action=skip reason=" + reason - + " cause=identity_missing"); + + " cause=identity_missing" + ); } } }; @@ -439,7 +467,8 @@ public void run() { "auth_refresh action=schedule reason=" + reason + " delay_ms=" - + timeDuration); + + timeDuration + ); } catch (Exception e) { releaseRefreshTask(refreshTask); if (timer != null) { @@ -451,19 +480,22 @@ public void run() { "auth_refresh action=error reason=" + reason + " cause=schedule_failed", - e); + e + ); } } private synchronized boolean claimRefreshTask( TimerTask task, - IterableAuthRefreshReason reason) { + IterableAuthRefreshReason reason + ) { if (scheduledRefreshTask != task) { IterableLogger.d( TAG, "auth_refresh action=ignore reason=" + reason - + " cause=stale_task"); + + " cause=stale_task" + ); return false; } @@ -524,10 +556,10 @@ private void checkAndHandleAuthRefresh() { } void clearRefreshTimer() { - clearRefreshTimer("explicit_clear"); + clearRefreshTimer(RefreshCancellationReason.EXPLICIT_CLEAR); } - private synchronized void clearRefreshTimer(String reason) { + private synchronized void clearRefreshTimer(RefreshCancellationReason reason) { IterableAuthRefreshReason cancelledReason = scheduledRefreshReason; if (scheduledRefreshTask != null) { scheduledRefreshTask.cancel(); @@ -544,7 +576,8 @@ private synchronized void clearRefreshTimer(String reason) { "auth_refresh action=cancel reason=" + cancelledReason + " cause=" - + reason); + + reason + ); } } @@ -567,7 +600,7 @@ public void onSwitchToBackground() { try { IterableLogger.d(TAG, "App switched to background - disabling auth token requests"); isInForeground = false; - clearRefreshTimer("app_backgrounded"); + clearRefreshTimer(RefreshCancellationReason.APP_BACKGROUNDED); } catch (Exception e) { IterableLogger.e(TAG, "Error while switching to background", e); } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java index 45728106d..8c66c885d 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java @@ -276,7 +276,8 @@ private static void handleJwtAuthRetry(IterableApiRequest iterableApiRequest) { authManager.scheduleAuthTokenRefresh( retryInterval, IterableAuthRefreshReason.JWT_401, - null); + null + ); } else { requestNewAuthTokenAndRetry(iterableApiRequest); } @@ -434,13 +435,14 @@ private static void requestNewAuthTokenAndRetry(IterableApiRequest iterableApiRe retryInterval, IterableAuthRefreshReason.JWT_401, data -> { - try { - String newAuthToken = data.getString("newAuthToken"); - retryRequestWithNewAuthToken(newAuthToken, iterableApiRequest); - } catch (JSONException e) { - e.printStackTrace(); - } - }); + try { + String newAuthToken = data.getString("newAuthToken"); + retryRequestWithNewAuthToken(newAuthToken, iterableApiRequest); + } catch (JSONException e) { + e.printStackTrace(); + } + } + ); } protected void setRetryCount(int count) { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java index b2ee90989..e390f3960 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java @@ -578,7 +578,8 @@ public void schedule(TimerTask task, long delay) { authManager.scheduleAuthTokenRefresh( 60000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null); + null + ); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java index 6cf7743dc..285828721 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java @@ -8,6 +8,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -43,14 +44,18 @@ public void setUp() { retryExecutor = mock(ScheduledExecutorService.class); scheduledTasks = new ArrayDeque<>(); - when(retryExecutor.schedule( - any(Runnable.class), - anyLong(), - eq(TimeUnit.MILLISECONDS))) - .thenAnswer(invocation -> { + when( + retryExecutor.schedule( + any(Runnable.class), + anyLong(), + eq(TimeUnit.MILLISECONDS) + ) + ).thenAnswer( + invocation -> { scheduledTasks.add(invocation.getArgument(0)); return mock(ScheduledFuture.class); - }); + } + ); IterableApi.sharedInstance.keychain = keychain; IterableApi.sharedInstance.authDataRestoreExecutor = retryExecutor; @@ -81,7 +86,8 @@ public void repeatedTimeoutsNeverRequestANewTokenWithoutConfirmedAbsence() { public void timeoutThenSuccessRestoresTokenWithoutCallingClientHandler() { doReturn( KeychainReadResult.TimedOut.INSTANCE, - new KeychainReadResult.Value(VALID_JWT)) + new KeychainReadResult.Value(VALID_JWT) + ) .when(keychain) .readAuthToken(); @@ -94,7 +100,8 @@ public void timeoutThenSuccessRestoresTokenWithoutCallingClientHandler() { assertTrue(authManager.isAuthTokenReady()); assertEquals( IterableAuthRefreshReason.TOKEN_EXPIRING, - authManager.scheduledRefreshReason); + authManager.scheduledRefreshReason + ); verify(authHandler, never()).onAuthTokenRequested(); } @@ -111,7 +118,8 @@ public void confirmedMissingTokenSchedulesOneReasonedRefresh() { assertFalse(authManager.isAuthTokenReady()); assertEquals( IterableAuthRefreshReason.STORED_TOKEN_MISSING, - authManager.scheduledRefreshReason); + authManager.scheduledRefreshReason + ); verify(authHandler, never()).onAuthTokenRequested(); } @@ -129,7 +137,8 @@ public void explicitTokenAfterConfirmedAbsenceUnblocksAuth() { assertTrue(authManager.isAuthTokenReady()); assertEquals( IterableAuthRefreshReason.TOKEN_EXPIRING, - authManager.scheduledRefreshReason); + authManager.scheduledRefreshReason + ); verify(authHandler, never()).onAuthTokenRequested(); } @@ -137,16 +146,24 @@ public void explicitTokenAfterConfirmedAbsenceUnblocksAuth() { public void explicitLoginWinsOverAQueuedRestoreRetry() { doReturn( KeychainReadResult.TimedOut.INSTANCE, - new KeychainReadResult.Value("old-token")) + new KeychainReadResult.Value("old-token") + ) .when(keychain) .readAuthToken(); initialize(); + IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); + AtomicReference tokenWhenAuthBecameReady = new AtomicReference<>(); + authManager.addAuthTokenReadyListener( + () -> tokenWhenAuthBecameReady.set(IterableApi.getInstance().getAuthToken()) + ); + IterableApi.getInstance().setEmail(NEW_EMAIL, VALID_JWT); runNext(); assertEquals(NEW_EMAIL, IterableApi.getInstance().getEmail()); assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); + assertEquals(VALID_JWT, tokenWhenAuthBecameReady.get()); } private void initialize() { @@ -156,7 +173,8 @@ private void initialize() { new IterableConfig.Builder() .setAutoPushRegistration(false) .setAuthHandler(authHandler) - .build()); + .build() + ); } private void runNext() { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java index 7e117a706..78f029dfd 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java @@ -34,14 +34,18 @@ public void setUp() { keychain = mock(IterableKeychain.class); retryExecutor = mock(ScheduledExecutorService.class); scheduledTasks = new ArrayDeque<>(); - when(retryExecutor.schedule( - any(Runnable.class), - anyLong(), - eq(TimeUnit.MILLISECONDS))) - .thenAnswer(invocation -> { + when( + retryExecutor.schedule( + any(Runnable.class), + anyLong(), + eq(TimeUnit.MILLISECONDS) + ) + ).thenAnswer( + invocation -> { scheduledTasks.add(invocation.getArgument(0)); return mock(ScheduledFuture.class); - }); + } + ); callback = new RecordingCallback(); restorer = new IterableAuthDataRestorer(keychain, retryExecutor); @@ -83,7 +87,8 @@ public void foregroundRetryCanRecoverAfterAnUnavailableCycle() { doReturn( KeychainReadResult.TimedOut.INSTANCE, KeychainReadResult.TimedOut.INSTANCE, - new KeychainReadResult.Value("stored-token")) + new KeychainReadResult.Value("stored-token") + ) .when(keychain) .readAuthToken(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java index af70ab0f6..46908dd48 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java @@ -9,6 +9,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -20,20 +21,28 @@ import static org.mockito.Mockito.when; public class IterableAuthRefreshOwnershipTest extends BaseTest { + private static final String VALID_JWT = + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9." + + "mYtgSqdUIxK8_RnYBTUP4cmpKw83aKi7cMiixF3qMB4"; + private IterableApi api; private IterableAuthManager authManager; + private IterableAuthHandler authHandler; private ExecutorService executor; @Before public void setUp() { api = mock(IterableApi.class); when(api.getEmail()).thenReturn("user@example.com"); + authHandler = mock(IterableAuthHandler.class); authManager = new IterableAuthManager( api, - mock(IterableAuthHandler.class), + authHandler, new RetryPolicy(3, 1, RetryPolicy.Type.LINEAR), - 60_000); + 60_000 + ); executor = mock(ExecutorService.class); authManager.executor = executor; } @@ -50,7 +59,8 @@ public void staleTaskCannotRunOrClearItsReplacement() { authManager.scheduleAuthTokenRefresh( 1000, IterableAuthRefreshReason.AUTH_HANDLER_RETRY, - null); + null + ); TimerTask staleTask = firstTimer.lastTask(); authManager.clearRefreshTimer(); @@ -59,7 +69,8 @@ public void staleTaskCannotRunOrClearItsReplacement() { authManager.scheduleAuthTokenRefresh( 2000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null); + null + ); TimerTask replacementTask = replacementTimer.lastTask(); staleTask.run(); @@ -67,7 +78,8 @@ public void staleTaskCannotRunOrClearItsReplacement() { assertSame(replacementTask, authManager.scheduledRefreshTask); assertEquals( IterableAuthRefreshReason.TOKEN_EXPIRING, - authManager.scheduledRefreshReason); + authManager.scheduledRefreshReason + ); verify(executor, never()).submit(any(Runnable.class)); replacementTask.run(); @@ -84,7 +96,8 @@ public void firingTaskReleasesOwnershipBeforeAnotherRefreshIsScheduled() { authManager.scheduleAuthTokenRefresh( 1000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null); + null + ); timer.lastTask().run(); assertNull(authManager.scheduledRefreshTask); @@ -92,12 +105,31 @@ public void firingTaskReleasesOwnershipBeforeAnotherRefreshIsScheduled() { authManager.scheduleAuthTokenRefresh( 2000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null); + null + ); assertEquals(2, timer.taskCount()); assertSame(timer.lastTask(), authManager.scheduledRefreshTask); } + @Test + public void generatedTokenIsStoredOnTheManagersApiInstance() { + IterableApi replacementSharedInstance = mock(IterableApi.class); + IterableApi.sharedInstance = replacementSharedInstance; + when(authHandler.onAuthTokenRequested()).thenReturn(VALID_JWT); + when(executor.submit(any(Runnable.class))).thenAnswer( + invocation -> { + invocation.getArgument(0).run(); + return mock(Future.class); + } + ); + + authManager.requestNewAuthToken(false, null); + + verify(api).setAuthToken(VALID_JWT); + verify(replacementSharedInstance, never()).setAuthToken(VALID_JWT); + } + private static class RetainingTimer extends Timer { private final List tasks = new ArrayList<>(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java index 980daa669..82c4961f4 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java @@ -235,7 +235,8 @@ public void backgroundCancelsTheScheduledRefresh() { assertNull("backgrounding must cancel the refresh timer", authManager.timer); assertNull( "backgrounding must release refresh ownership", - authManager.scheduledRefreshTask); + authManager.scheduledRefreshTask + ); } @Test @@ -277,7 +278,9 @@ public void concurrentSchedulingArmsOnlyOneRefresh() throws Exception { () -> authManager.scheduleAuthTokenRefresh( 60_000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null)); + null + ) + ); assertEquals(1, timer.liveTaskCount()); } @@ -368,7 +371,8 @@ private CountingTimer armObservableRefresh() { authManager.scheduleAuthTokenRefresh( 60_000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null); + null + ); assertEquals(1, timer.liveTaskCount()); return timer; } From fa020023c16dff8f7b16c079ad6d0952ca536959 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Wed, 19 Aug 2026 12:44:56 +0100 Subject: [PATCH 07/11] [SDK-547] Discard auth results that outlived their identity An auth token can arrive from two async sources that both outlive the identity that asked for them: the blocking onAuthTokenRequested callback and an encrypted-storage read. Neither store site checked who was signed in, so a token minted for the previous user could be installed on the current session, and a second initialize() left an unreachable restorer that could overwrite a fresh login's token. Track the signed-in identity in an AtomicReference and compare it at both store sites. Atomics rather than a lock: IterableAuthDataRestorer invokes its callback while holding its own monitor and that callback re-enters this class, so taking the manager's monitor around a restore would invert the established restorer -> manager order. The handler snapshot is taken immediately before onAuthTokenRequested, not at submit time. An identity change before the handler runs is served by that request, because pendingAuth makes the new login reuse it rather than start its own; discarding there would leave the new user with no token and nothing in flight. A discarded result still clears pendingAuth and replays any refresh deferred behind it, since both are otherwise released only by a stored result. Also gate useExplicitAuthToken on the token actually changing. Its caller compares by reference, so an equal-but-distinct String - a token read from disk or JSON - reached it on every repeated login and replaced the refresh timer while downgrading INVALID to UNKNOWN, marking a token ready that a 401 had just rejected. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 + .../iterableapi/IterableAuthManager.java | 126 +++++++- .../IterableAuthIdentityGuardTest.java | 299 ++++++++++++++++++ 3 files changed, 415 insertions(+), 12 deletions(-) create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index b9d225ee9..e0b471af7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fixed a race in JWT auth token refresh scheduling that could leave multiple overlapping refresh timers running. The pending refresh task is now the single source of ownership, and cancelled or replaced tasks cannot execute or clear their replacement. Refresh scheduling also records an explicit reason, such as token expiration, a 401 retry, or a missing stored token. - 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 on the next launch. Crypto timeouts now preserve stored data and encryption state; timed-out operations are cancelled so they do not block later reads or writes. +- Fixed a JWT auth token belonging to a previous user being installed on the current session. A token fetched from `IterableAuthHandler.onAuthTokenRequested()`, or read back from encrypted storage, can arrive after the app has signed in a different user or called `initialize()` again; such a result is now discarded instead of stored. A login that happens *before* the handler is invoked still reuses the request already in flight, so no additional `onAuthTokenRequested()` call is introduced. +- Fixed `setEmail(email, authToken)` / `setUserId(userId, authToken)` replacing the refresh timer and clearing the invalid-token state when called repeatedly with an equal token value. A repeated login could postpone an expiration refresh indefinitely and mark a token ready that a 401 had just rejected. - Fixed a timed-out stored-token read being mistaken for a confirmed missing JWT. Transient token-read timeouts are retried off the caller thread, and `IterableAuthHandler.onAuthTokenRequested()` is invoked only after a completed read confirms that the stored token is absent. If storage remains unavailable, JWT-required work stays blocked and restoration is retried when the app returns to the foreground. ## [3.10.0] diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index 4537c0103..beb5cc4b1 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -15,6 +15,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicReference; public class IterableAuthManager implements IterableActivityMonitor.AppStateCallback { private static final String TAG = "IterableAuth"; @@ -41,6 +42,25 @@ private enum RefreshCancellationReason { APP_BACKGROUNDED } + /** + * A snapshot of "who was signed in" when async auth work began. Auth results arrive from a + * developer callback and from encrypted storage, both of which can outlive the identity that + * asked for them; a result carrying a superseded identity must be dropped rather than stored. + * Comparison is by reference — {@code version} only exists to make log lines readable. + */ + private static final class AuthIdentity { + private final int version; + + AuthIdentity(int version) { + this.version = version; + } + + @Override + public String toString() { + return "identity#" + version; + } + } + /** * Listener interface for components that need to react when a new auth token is ready. */ @@ -58,8 +78,9 @@ interface AuthTokenReadyListener { volatile TimerTask scheduledRefreshTask; @VisibleForTesting volatile IterableAuthRefreshReason scheduledRefreshReason; - @Nullable - private volatile IterableAuthDataRestorer authDataRestorer; + private final AtomicReference authDataRestorer = new AtomicReference<>(); + private final AtomicReference currentIdentity = + new AtomicReference<>(new AuthIdentity(0)); private boolean hasFailedPriorAuth; private boolean pendingAuth; private boolean requiresAuthRefresh; @@ -101,6 +122,10 @@ boolean isAuthTokenReady() { if (authHandler == null) { return true; } + // A signed-out user has no token to restore, so RESTORING would block work indefinitely. + if (authState == AuthState.RESTORING && !hasIdentity()) { + return true; + } return isReadyState(authState); } @@ -132,6 +157,51 @@ private boolean isReadyState(AuthState state) { return state == AuthState.VALID || state == AuthState.UNKNOWN; } + private boolean hasIdentity() { + return getEmailOrUserId() != null; + } + + /** + * Supersedes every auth result already in flight; returns the identity that replaces them. + * Only call this where a replacement token or refresh is about to be armed — superseding + * without a successor leaves auth with nothing pending. + */ + private AuthIdentity startNewIdentity() { + AuthIdentity previous; + AuthIdentity next; + do { + previous = currentIdentity.get(); + next = new AuthIdentity(previous.version + 1); + } while (!currentIdentity.compareAndSet(previous, next)); + return next; + } + + private boolean isStillCurrent(AuthIdentity identity) { + return currentIdentity.get() == identity; + } + + /** + * A discarded result must release everything the stored result would have released: + * {@code pendingAuth}, because {@link #requestNewAuthToken} refuses every future request while + * it is set, and any refresh deferred while this request was in flight, which otherwise leaves + * the new identity with no token and nothing scheduled. + */ + private boolean wasSupersededSince(AuthIdentity identity) { + if (isStillCurrent(identity)) { + return false; + } + IterableLogger.d( + TAG, + "auth_token action=discard started_as=" + + identity + + " now=" + + currentIdentity.get() + ); + pendingAuth = false; + reSyncAuth(); + return true; + } + @Nullable String restoreAuthToken( IterableKeychain keychain, @@ -139,16 +209,29 @@ String restoreAuthToken( ) { KeychainReadResult initialRead = keychain.readAuthToken(); setAuthState(AuthState.RESTORING); - authDataRestorer = retryExecutor == null + final AuthIdentity identity = startNewIdentity(); + final IterableAuthDataRestorer restorer = retryExecutor == null ? new IterableAuthDataRestorer(keychain) : new IterableAuthDataRestorer(keychain, retryExecutor); - authDataRestorer.restore( + // Published before restore() because a successful initial read completes inline. + authDataRestorer.set(restorer); + restorer.restore( initialRead, new IterableAuthDataRestorer.Callback() { @Override public void onAuthTokenRestored(@Nullable String authToken) { + authDataRestorer.compareAndSet(restorer, null); + if (!isStillCurrent(identity)) { + IterableLogger.d( + TAG, + "auth_restore action=discard started_as=" + + identity + + " now=" + + currentIdentity.get() + ); + return; + } api.setRestoredAuthToken(authToken); - authDataRestorer = null; handleRestoredAuthToken(authToken); } @@ -178,16 +261,22 @@ private void handleRestoredAuthToken(@Nullable String authToken) { } void cancelAuthTokenRestore(String reason) { - if (authDataRestorer != null) { - authDataRestorer.cancel(reason); - authDataRestorer = null; + IterableAuthDataRestorer restorer = authDataRestorer.getAndSet(null); + if (restorer != null) { + restorer.cancel(reason); } } void useExplicitAuthToken(String authToken) { cancelAuthTokenRestore("explicit_token"); + boolean tokenChanged = authToken == null + ? api.getAuthToken() != null + : !authToken.equalsIgnoreCase(api.getAuthToken()); api.setAuthToken(authToken); - if (authHandler != null) { + // Re-arming on an unchanged token would postpone an expiry refresh indefinitely and + // downgrade INVALID to UNKNOWN, marking a token ready that a 401 just rejected. + if (authHandler != null && tokenChanged) { + startNewIdentity(); setAuthState(AuthState.UNKNOWN); queueExpirationRefresh(authToken); } @@ -216,6 +305,7 @@ void reset() { void resetForIdentityChange() { cancelAuthTokenRestore("identity_changed"); + startNewIdentity(); if (authHandler != null) { setAuthState(AuthState.RESTORING); } @@ -260,6 +350,7 @@ public synchronized void requestNewAuthToken( executor.submit(new Runnable() { @Override public void run() { + AuthIdentity identity = null; try { if (isLastAuthTokenValid && !shouldIgnoreRetryPolicy) { // if some JWT retry had valid token it will not fetch the auth token again from developer function @@ -275,12 +366,22 @@ public void run() { return; } + // Snapshot here, not at submit time: an identity change before the + // handler runs is served by this very request, because pendingAuth + // makes the new login reuse it instead of starting its own. + identity = currentIdentity.get(); final String authToken = authHandler.onAuthTokenRequested(); pendingAuth = false; retryCount++; + if (wasSupersededSince(identity)) { + return; + } handleAuthTokenSuccess(authToken, successCallback); } catch (final Exception e) { retryCount++; + if (identity != null && wasSupersededSince(identity)) { + return; + } handleAuthTokenFailure(e); } } @@ -441,7 +542,7 @@ public void run() { } IterableLogger.d(TAG, "auth_refresh action=fire reason=" + reason); - if (api.getEmail() != null || api.getUserId() != null) { + if (hasIdentity()) { requestNewAuthToken( false, successCallback, @@ -547,7 +648,7 @@ private static String getJson(String strEncoded) throws UnsupportedEncodingExcep */ private void checkAndHandleAuthRefresh() { // First, check if current auth token needs refresh based on expiration - if (api.getEmail() != null || api.getUserId() != null) { + if (hasIdentity()) { String currentAuthToken = api.getAuthToken(); queueExpirationRefresh(currentAuthToken); } else { @@ -586,7 +687,8 @@ public void onSwitchToForeground() { try { IterableLogger.d(TAG, "App switched to foreground - enabling auth token requests"); isInForeground = true; - if (authDataRestorer != null && authDataRestorer.resumeIfUnresolved()) { + IterableAuthDataRestorer restorer = authDataRestorer.get(); + if (restorer != null && restorer.resumeIfUnresolved()) { return; } checkAndHandleAuthRefresh(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java new file mode 100644 index 000000000..d171a532c --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java @@ -0,0 +1,299 @@ +package com.iterable.iterableapi; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.List; +import java.util.Queue; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +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.mockito.Mockito.when; + +/** + * Auth results arrive asynchronously from a developer callback and from encrypted storage. Both can + * outlive the identity that asked for them, and storing a superseded result puts one user's token on + * another user's session. + */ +public class IterableAuthIdentityGuardTest extends BaseTest { + private static final String JWT_HEADER_AND_PAYLOAD = + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9."; + // Only the payload segment is parsed, so a distinct signature is enough to make a distinct token. + private static final String TOKEN_A = JWT_HEADER_AND_PAYLOAD + "signature-for-user-a"; + private static final String TOKEN_B = JWT_HEADER_AND_PAYLOAD + "signature-for-user-b"; + + private IterableApi api; + private IterableAuthHandler authHandler; + private IterableAuthManager authManager; + private IterableKeychain keychain; + private ScheduledExecutorService retryExecutor; + private Queue restoreRetries; + private List submittedAuthWork; + + @Before + public void setUp() { + api = mock(IterableApi.class); + when(api.getEmail()).thenReturn("user-a@example.com"); + authHandler = mock(IterableAuthHandler.class); + + authManager = new IterableAuthManager( + api, + authHandler, + new RetryPolicy(3, 1, RetryPolicy.Type.LINEAR), + 60_000 + ); + authManager.timer = new RetainingTimer(); + + submittedAuthWork = new ArrayList<>(); + ExecutorService executor = mock(ExecutorService.class); + when(executor.submit(any(Runnable.class))).thenAnswer( + invocation -> { + submittedAuthWork.add(invocation.getArgument(0)); + return mock(Future.class); + } + ); + authManager.executor = executor; + + keychain = mock(IterableKeychain.class); + retryExecutor = mock(ScheduledExecutorService.class); + restoreRetries = new ArrayDeque<>(); + when( + retryExecutor.schedule( + any(Runnable.class), + anyLong(), + eq(TimeUnit.MILLISECONDS) + ) + ).thenAnswer( + invocation -> { + restoreRetries.add(invocation.getArgument(0)); + return mock(ScheduledFuture.class); + } + ); + } + + @After + public void tearDown() { + authManager.clearRefreshTimer(); + } + + @Test + public void anOrphanedRestorerCannotStoreItsTokenAfterANewRestoreBegins() { + doReturn(KeychainReadResult.TimedOut.INSTANCE).when(keychain).readAuthToken(); + authManager.restoreAuthToken(keychain, retryExecutor); + assertEquals(1, restoreRetries.size()); + + doReturn(new KeychainReadResult.Value(TOKEN_B)).when(keychain).readAuthToken(); + authManager.restoreAuthToken(keychain, retryExecutor); + verify(api).setRestoredAuthToken(TOKEN_B); + + doReturn(new KeychainReadResult.Value(TOKEN_A)).when(keychain).readAuthToken(); + restoreRetries.remove().run(); + + verify(api, never()).setRestoredAuthToken(TOKEN_A); + } + + /** + * {@code onAuthTokenRequested} is a blocking developer callback, so the user can change while it + * runs. The token it returns belongs to the user who was signed in when it was called. + */ + @Test + public void aHandlerResultIsDiscardedWhenTheIdentityChangesWhileTheHandlerRuns() { + when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { + authManager.resetForIdentityChange(); + when(api.getEmail()).thenReturn("user-b@example.com"); + return TOKEN_A; + }); + authManager.requestNewAuthToken(false, null); + assertEquals(1, submittedAuthWork.size()); + + submittedAuthWork.get(0).run(); + + verify(api, never()).setAuthToken(TOKEN_A); + verify(authHandler, never()).onTokenRegistrationSuccessful(anyString()); + } + + /** + * A request that arrives while this one is in flight is deferred rather than started, and only + * a stored result replays it. Discarding must replay it too, or the new identity is left with no + * token and nothing scheduled. + */ + @Test + public void discardingAStaleResultStillHonoursTheRefreshDeferredBehindIt() { + when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { + authManager.requestNewAuthToken(false, null); + authManager.resetForIdentityChange(); + when(api.getEmail()).thenReturn("user-b@example.com"); + return TOKEN_A; + }); + authManager.requestNewAuthToken(false, null); + + submittedAuthWork.get(0).run(); + + verify(api, never()).setAuthToken(TOKEN_A); + assertNotNull( + "the deferred refresh must be armed for the identity that replaced this one", + authManager.scheduledRefreshTask + ); + } + + /** + * The mirror of the test above: an identity change before the handler is called must not discard + * anything, because {@code pendingAuth} makes the new login reuse this request rather than start + * its own — discarding here would leave the new user with no token and nothing in flight. + */ + @Test + public void anIdentityChangeBeforeTheHandlerRunsIsServedByTheRequestInFlight() { + when(authHandler.onAuthTokenRequested()).thenReturn(TOKEN_A); + authManager.requestNewAuthToken(false, null); + + authManager.resetForIdentityChange(); + when(api.getEmail()).thenReturn("user-b@example.com"); + submittedAuthWork.get(0).run(); + + verify(api).setAuthToken(TOKEN_A); + } + + /** + * A discarded result must still release {@code pendingAuth}, or every later request is refused + * for the life of the process. The throwing handler matters: on the success path the flag is + * already cleared before the discard, so only the failure path can leak it. + */ + @Test + public void discardingAStaleFailureLeavesAuthAbleToRequestAgain() { + when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { + authManager.resetForIdentityChange(); + throw new RuntimeException("boom"); + }); + authManager.requestNewAuthToken(false, null); + submittedAuthWork.get(0).run(); + + authManager.requestNewAuthToken(false, null); + + assertEquals(2, submittedAuthWork.size()); + } + + /** + * An orphaned restorer completing must not clear the field out from under the restore that + * replaced it — doing so loses the foreground retry that recovers an unresolved read. + */ + @Test + public void anOrphanedRestorerDoesNotEvictTheRestoreThatReplacedIt() { + doReturn(KeychainReadResult.TimedOut.INSTANCE).when(keychain).readAuthToken(); + authManager.restoreAuthToken(keychain, retryExecutor); + authManager.restoreAuthToken(keychain, retryExecutor); + assertEquals(2, restoreRetries.size()); + + doReturn(new KeychainReadResult.Value(TOKEN_A)).when(keychain).readAuthToken(); + restoreRetries.remove().run(); + + authManager.authRetryPolicy = new RetryPolicy(3, 60_000, RetryPolicy.Type.LINEAR); + authManager.onSwitchToForeground(); + + assertNull(authManager.scheduledRefreshTask); + } + + @Test + public void aHandlerFailureForASupersededIdentityIsNotReported() { + when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { + authManager.resetForIdentityChange(); + throw new RuntimeException("boom"); + }); + authManager.requestNewAuthToken(false, null); + + submittedAuthWork.get(0).run(); + + verify(authHandler, never()).onAuthFailure(any(AuthFailure.class)); + } + + @Test + public void anInFlightHandlerResultStillLandsWhenTheIdentityIsUnchanged() { + when(authHandler.onAuthTokenRequested()).thenReturn(TOKEN_A); + authManager.requestNewAuthToken(false, null); + + submittedAuthWork.get(0).run(); + + verify(api).setAuthToken(TOKEN_A); + verify(authHandler).onTokenRegistrationSuccessful(TOKEN_A); + } + + @Test + public void repeatingAnEqualExplicitTokenDoesNotRearmOrUnblockAnInvalidToken() { + when(api.getAuthToken()).thenReturn(TOKEN_A); + authManager.setAuthTokenInvalid(); + + authManager.useExplicitAuthToken(new String(TOKEN_A)); + + assertEquals(IterableAuthManager.AuthState.INVALID, authManager.getAuthState()); + assertNull(authManager.scheduledRefreshTask); + } + + @Test + public void aGenuinelyNewExplicitTokenStillArmsARefresh() { + when(api.getAuthToken()).thenReturn(TOKEN_A); + authManager.setAuthTokenInvalid(); + + authManager.useExplicitAuthToken(TOKEN_B); + + assertEquals(IterableAuthManager.AuthState.UNKNOWN, authManager.getAuthState()); + assertNotNull(authManager.scheduledRefreshTask); + } + + @Test + public void restoringWithoutAnIdentityDoesNotBlockWork() { + when(api.getEmail()).thenReturn(null); + when(api.getUserId()).thenReturn(null); + + authManager.resetForIdentityChange(); + + assertEquals(IterableAuthManager.AuthState.RESTORING, authManager.getAuthState()); + assertTrue(authManager.isAuthTokenReady()); + } + + @Test + public void restoringWithAnIdentityStillBlocksWork() { + when(api.getEmail()).thenReturn("user-b@example.com"); + + authManager.resetForIdentityChange(); + + assertFalse(authManager.isAuthTokenReady()); + } + + private static class RetainingTimer extends Timer { + RetainingTimer() { + super(true); + super.cancel(); + } + + @Override + public void schedule(TimerTask task, long delay) { + // Retain nothing; tests only assert on the manager's ownership field. + } + + @Override + public void cancel() { + // Keep the timer usable across a clearRefreshTimer() call. + } + } +} From ed9c6b75624e63a3aaef05a561d8038452dcefd1 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Wed, 19 Aug 2026 12:44:57 +0100 Subject: [PATCH 08/11] [SDK-547] Exempt IterableApi from the file-length check IterableApi is the SDK public entry point and grows with the API surface, so the 2000-line default is fighting the architecture rather than protecting anything. Suppress the check for that one file instead of raising the limit, so every other file keeps the guard. Co-Authored-By: Claude Opus 5 --- checkstyle.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/checkstyle.xml b/checkstyle.xml index f0fada454..3bbf7bcad 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -6,6 +6,11 @@ + + + + + From fc688ae226874eb9ca7c15e71fba0d5ea23ffbf7 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Thu, 20 Aug 2026 10:33:11 +0100 Subject: [PATCH 09/11] [SDK-547] Keep auth callbacks bound to identity --- .../iterableapi/IterableAuthManager.java | 194 ++++++++++++------ .../IterableAuthRequestCoordinator.java | 154 ++++++++++++++ .../iterableapi/IterableApiAuthTests.java | 2 +- .../IterableAuthIdentityGuardTest.java | 30 +-- .../IterableAuthRequestCoordinatorTest.java | 150 ++++++++++++++ 5 files changed, 454 insertions(+), 76 deletions(-) create mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index beb5cc4b1..0da973ec2 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -81,8 +81,9 @@ interface AuthTokenReadyListener { private final AtomicReference authDataRestorer = new AtomicReference<>(); private final AtomicReference currentIdentity = new AtomicReference<>(new AuthIdentity(0)); + private final IterableAuthRequestCoordinator authRequestCoordinator = + new IterableAuthRequestCoordinator<>(); private boolean hasFailedPriorAuth; - private boolean pendingAuth; private boolean requiresAuthRefresh; RetryPolicy authRetryPolicy; boolean pauseAuthRetry; @@ -167,6 +168,7 @@ private boolean hasIdentity() { * without a successor leaves auth with nothing pending. */ private AuthIdentity startNewIdentity() { + authRequestCoordinator.clearQueued(); AuthIdentity previous; AuthIdentity next; do { @@ -180,16 +182,7 @@ private boolean isStillCurrent(AuthIdentity identity) { return currentIdentity.get() == identity; } - /** - * A discarded result must release everything the stored result would have released: - * {@code pendingAuth}, because {@link #requestNewAuthToken} refuses every future request while - * it is set, and any refresh deferred while this request was in flight, which otherwise leaves - * the new identity with no token and nothing scheduled. - */ - private boolean wasSupersededSince(AuthIdentity identity) { - if (isStillCurrent(identity)) { - return false; - } + private void logSupersededRequest(AuthIdentity identity) { IterableLogger.d( TAG, "auth_token action=discard started_as=" @@ -197,9 +190,6 @@ private boolean wasSupersededSince(AuthIdentity identity) { + " now=" + currentIdentity.get() ); - pendingAuth = false; - reSyncAuth(); - return true; } @Nullable @@ -342,61 +332,140 @@ public synchronized void requestNewAuthToken( } if (authHandler != null) { - if (!pendingAuth) { - if (!(this.hasFailedPriorAuth && hasFailedPriorAuth)) { - this.hasFailedPriorAuth = hasFailedPriorAuth; - pendingAuth = true; - - executor.submit(new Runnable() { - @Override - public void run() { - AuthIdentity identity = null; - try { - if (isLastAuthTokenValid && !shouldIgnoreRetryPolicy) { - // if some JWT retry had valid token it will not fetch the auth token again from developer function - handleAuthTokenSuccess(api.getAuthToken(), successCallback); - pendingAuth = false; - return; - } - - // Only request new auth token if app is in foreground - if (!isInForeground) { - IterableLogger.w(TAG, "Auth token request skipped - app is in background"); - pendingAuth = false; - return; - } - - // Snapshot here, not at submit time: an identity change before the - // handler runs is served by this very request, because pendingAuth - // makes the new login reuse it instead of starting its own. - identity = currentIdentity.get(); - final String authToken = authHandler.onAuthTokenRequested(); - pendingAuth = false; - retryCount++; - if (wasSupersededSince(identity)) { - return; - } - handleAuthTokenSuccess(authToken, successCallback); - } catch (final Exception e) { - retryCount++; - if (identity != null && wasSupersededSince(identity)) { - return; - } - handleAuthTokenFailure(e); - } - } - }); - } - } else if (!hasFailedPriorAuth) { - //setFlag to resync auth after current auth returns - requiresAuthRefresh = true; + if (this.hasFailedPriorAuth && hasFailedPriorAuth) { + return; } + IterableAuthRequestCoordinator.EnqueueResult enqueueResult = + authRequestCoordinator.enqueue( + currentIdentity.get(), + successCallback, + hasFailedPriorAuth, + shouldIgnoreRetryPolicy + ); + if (enqueueResult.getStatus() + == IterableAuthRequestCoordinator.EnqueueStatus.STARTED) { + submitAuthRequest(enqueueResult.getRequestToStart()); + } else if (enqueueResult.getStatus() + == IterableAuthRequestCoordinator.EnqueueStatus + .ALREADY_ACTIVE_FOR_IDENTITY && !hasFailedPriorAuth) { + requiresAuthRefresh = true; + } } else { api.setAuthToken(null, true); } } + private void submitAuthRequest( + IterableAuthRequestCoordinator.Request request + ) { + hasFailedPriorAuth = request.hasFailedPriorAuth(); + executor.submit(() -> executeAuthRequest(request)); + } + + private void executeAuthRequest( + IterableAuthRequestCoordinator.Request request + ) { + if (!authRequestCoordinator.isCurrent(request, currentIdentity.get())) { + finishSupersededRequest(request); + return; + } + + if (isLastAuthTokenValid && !request.shouldIgnoreRetryPolicy()) { + completeAuthRequestWithToken(request, api.getAuthToken()); + return; + } + + if (!isInForeground) { + IterableLogger.w(TAG, "Auth token request skipped - app is in background"); + IterableAuthRequestCoordinator.Completion completion = + authRequestCoordinator.complete(request, currentIdentity.get()); + if (!completion.isResultAccepted()) { + finishSupersededRequest(request, completion); + return; + } + submitNextAuthRequest(completion); + return; + } + + String authToken; + try { + authToken = authHandler.onAuthTokenRequested(); + retryCount++; + } catch (Exception e) { + retryCount++; + completeAuthRequestWithFailure(request, e); + return; + } + completeAuthRequestWithToken(request, authToken); + } + + private void completeAuthRequestWithToken( + IterableAuthRequestCoordinator.Request request, + String authToken + ) { + IterableAuthRequestCoordinator.Completion completion = + authRequestCoordinator.complete(request, currentIdentity.get()); + if (!completion.isResultAccepted()) { + finishSupersededRequest(request, completion); + return; + } + + try { + handleAuthTokenSuccess(authToken, request.getSuccessCallback()); + } catch (Exception e) { + retryCount++; + handleAuthTokenFailure(e); + } finally { + submitNextAuthRequest(completion); + } + } + + private void completeAuthRequestWithFailure( + IterableAuthRequestCoordinator.Request request, + Exception exception + ) { + IterableAuthRequestCoordinator.Completion completion = + authRequestCoordinator.complete(request, currentIdentity.get()); + if (!completion.isResultAccepted()) { + finishSupersededRequest(request, completion); + return; + } + + try { + handleAuthTokenFailure(exception); + } finally { + submitNextAuthRequest(completion); + } + } + + private void finishSupersededRequest( + IterableAuthRequestCoordinator.Request request + ) { + IterableAuthRequestCoordinator.Completion completion = + authRequestCoordinator.complete(request, currentIdentity.get()); + finishSupersededRequest(request, completion); + } + + private void finishSupersededRequest( + IterableAuthRequestCoordinator.Request request, + IterableAuthRequestCoordinator.Completion completion + ) { + logSupersededRequest(request.getIdentity()); + reSyncAuth(); + submitNextAuthRequest(completion); + } + + private void submitNextAuthRequest( + IterableAuthRequestCoordinator.Completion completion + ) { + IterableAuthRequestCoordinator.Request nextRequest = + completion.getNextRequest(); + if (nextRequest != null) { + submitAuthRequest(nextRequest); + } + } + private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHandler successCallback) { if (authToken != null) { // Token obtained but not yet verified by a request. Storing it before changing state @@ -426,7 +495,6 @@ private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHand private void handleAuthTokenFailure(Throwable throwable) { IterableLogger.e(TAG, "Error while requesting Auth Token", throwable); handleAuthFailure(null, AuthFailureReason.AUTH_TOKEN_GENERATION_ERROR); - pendingAuth = false; scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.AUTH_HANDLER_RETRY, diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java new file mode 100644 index 000000000..d41321644 --- /dev/null +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java @@ -0,0 +1,154 @@ +package com.iterable.iterableapi; + +import androidx.annotation.Nullable; + +/** + * Serializes auth-token requests while keeping each callback bound to the identity that requested + * it. Identity comparison is by reference so a logout and login with the same value still creates + * a distinct auth lifecycle. + */ +class IterableAuthRequestCoordinator { + enum EnqueueStatus { + STARTED, + QUEUED_FOR_NEW_IDENTITY, + ALREADY_ACTIVE_FOR_IDENTITY, + IGNORED_FAILED_RETRY + } + + static final class Request { + private final I identity; + @Nullable + private final IterableHelper.SuccessHandler successCallback; + private final boolean hasFailedPriorAuth; + private final boolean shouldIgnoreRetryPolicy; + + Request( + I identity, + @Nullable IterableHelper.SuccessHandler successCallback, + boolean hasFailedPriorAuth, + boolean shouldIgnoreRetryPolicy + ) { + this.identity = identity; + this.successCallback = successCallback; + this.hasFailedPriorAuth = hasFailedPriorAuth; + this.shouldIgnoreRetryPolicy = shouldIgnoreRetryPolicy; + } + + I getIdentity() { + return identity; + } + + @Nullable + IterableHelper.SuccessHandler getSuccessCallback() { + return successCallback; + } + + boolean hasFailedPriorAuth() { + return hasFailedPriorAuth; + } + + boolean shouldIgnoreRetryPolicy() { + return shouldIgnoreRetryPolicy; + } + } + + static final class EnqueueResult { + private final EnqueueStatus status; + @Nullable + private final Request requestToStart; + + EnqueueResult(EnqueueStatus status, @Nullable Request requestToStart) { + this.status = status; + this.requestToStart = requestToStart; + } + + EnqueueStatus getStatus() { + return status; + } + + @Nullable + Request getRequestToStart() { + return requestToStart; + } + } + + static final class Completion { + private final boolean resultAccepted; + @Nullable + private final Request nextRequest; + + Completion(boolean resultAccepted, @Nullable Request nextRequest) { + this.resultAccepted = resultAccepted; + this.nextRequest = nextRequest; + } + + boolean isResultAccepted() { + return resultAccepted; + } + + @Nullable + Request getNextRequest() { + return nextRequest; + } + } + + @Nullable + private Request activeRequest; + @Nullable + private Request queuedRequest; + + synchronized EnqueueResult enqueue( + I identity, + @Nullable IterableHelper.SuccessHandler successCallback, + boolean hasFailedPriorAuth, + boolean shouldIgnoreRetryPolicy + ) { + Request request = new Request<>( + identity, + successCallback, + hasFailedPriorAuth, + shouldIgnoreRetryPolicy + ); + if (activeRequest == null) { + activeRequest = request; + return new EnqueueResult<>(EnqueueStatus.STARTED, request); + } + if (hasFailedPriorAuth) { + return new EnqueueResult<>(EnqueueStatus.IGNORED_FAILED_RETRY, null); + } + if (activeRequest.getIdentity() == identity) { + return new EnqueueResult<>( + EnqueueStatus.ALREADY_ACTIVE_FOR_IDENTITY, + null + ); + } + + queuedRequest = request; + return new EnqueueResult<>(EnqueueStatus.QUEUED_FOR_NEW_IDENTITY, null); + } + + synchronized boolean isCurrent(Request request, I currentIdentity) { + return activeRequest == request && request.getIdentity() == currentIdentity; + } + + synchronized Completion complete(Request request, I currentIdentity) { + if (activeRequest != request) { + return new Completion<>(false, null); + } + + boolean resultAccepted = request.getIdentity() == currentIdentity; + activeRequest = null; + + Request nextRequest = null; + if (queuedRequest != null && queuedRequest.getIdentity() == currentIdentity) { + nextRequest = queuedRequest; + activeRequest = nextRequest; + } + queuedRequest = null; + return new Completion<>(resultAccepted, nextRequest); + } + + synchronized void clearQueued() { + queuedRequest = null; + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java index e390f3960..af92eed85 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java @@ -546,7 +546,7 @@ public void testForegroundWithValidTokenDoesNotRequestNewToken() throws Exceptio // source of truth. // // We drive scheduleAuthTokenRefresh directly rather than requestNewAuthToken: the executor is - // not injectable (see @Ignore'd tests above) and its pendingAuth guard serializes calls, + // not injectable (see @Ignore'd tests above) and its auth request coordinator serializes calls, // which would hide the scheduling race we're targeting. @Test public void testConcurrentScheduleAuthTokenRefreshSchedulesOnlyOneTimer() throws Exception { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java index d171a532c..c3f394b74 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java @@ -158,27 +158,33 @@ public void discardingAStaleResultStillHonoursTheRefreshDeferredBehindIt() { ); } - /** - * The mirror of the test above: an identity change before the handler is called must not discard - * anything, because {@code pendingAuth} makes the new login reuse this request rather than start - * its own — discarding here would leave the new user with no token and nothing in flight. - */ + /** A queued login must keep its own callback when it replaces a superseded request. */ @Test - public void anIdentityChangeBeforeTheHandlerRunsIsServedByTheRequestInFlight() { - when(authHandler.onAuthTokenRequested()).thenReturn(TOKEN_A); - authManager.requestNewAuthToken(false, null); + public void anIdentityChangeBeforeTheHandlerRunsUsesTheNewIdentityCallback() { + IterableHelper.SuccessHandler callbackA = + mock(IterableHelper.SuccessHandler.class); + IterableHelper.SuccessHandler callbackB = + mock(IterableHelper.SuccessHandler.class); + when(authHandler.onAuthTokenRequested()).thenReturn(TOKEN_B); + authManager.requestNewAuthToken(false, callbackA); authManager.resetForIdentityChange(); when(api.getEmail()).thenReturn("user-b@example.com"); + authManager.requestNewAuthToken(false, callbackB); + assertEquals(1, submittedAuthWork.size()); + submittedAuthWork.get(0).run(); + assertEquals(2, submittedAuthWork.size()); + submittedAuthWork.get(1).run(); - verify(api).setAuthToken(TOKEN_A); + verify(api).setAuthToken(TOKEN_B); + verify(callbackA, never()).onSuccess(any()); + verify(callbackB).onSuccess(any()); } /** - * A discarded result must still release {@code pendingAuth}, or every later request is refused - * for the life of the process. The throwing handler matters: on the success path the flag is - * already cleared before the discard, so only the failure path can leak it. + * A discarded result must still release coordinator ownership, or every later request is + * refused for the life of the process. */ @Test public void discardingAStaleFailureLeavesAuthAbleToRequestAgain() { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java new file mode 100644 index 000000000..cf9397a84 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java @@ -0,0 +1,150 @@ +package com.iterable.iterableapi; + +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +public class IterableAuthRequestCoordinatorTest { + private IterableAuthRequestCoordinator coordinator; + private Object identityA; + private Object identityB; + + @Before + public void setUp() { + coordinator = new IterableAuthRequestCoordinator<>(); + identityA = new Object(); + identityB = new Object(); + } + + @Test + public void firstRequestStartsImmediately() { + IterableHelper.SuccessHandler callback = mock(IterableHelper.SuccessHandler.class); + + IterableAuthRequestCoordinator.EnqueueResult result = + coordinator.enqueue(identityA, callback, false, true); + + assertEquals( + IterableAuthRequestCoordinator.EnqueueStatus.STARTED, + result.getStatus() + ); + assertSame(callback, result.getRequestToStart().getSuccessCallback()); + assertFalse(result.getRequestToStart().hasFailedPriorAuth()); + assertTrue(result.getRequestToStart().shouldIgnoreRetryPolicy()); + } + + @Test + public void sameIdentityKeepsTheActiveRequest() { + IterableHelper.SuccessHandler firstCallback = + mock(IterableHelper.SuccessHandler.class); + IterableHelper.SuccessHandler secondCallback = + mock(IterableHelper.SuccessHandler.class); + IterableAuthRequestCoordinator.Request activeRequest = + coordinator.enqueue( + identityA, + firstCallback, + false, + true + ).getRequestToStart(); + + IterableAuthRequestCoordinator.EnqueueResult result = + coordinator.enqueue(identityA, secondCallback, false, false); + + assertEquals( + IterableAuthRequestCoordinator.EnqueueStatus.ALREADY_ACTIVE_FOR_IDENTITY, + result.getStatus() + ); + assertNull(result.getRequestToStart()); + assertTrue(coordinator.complete(activeRequest, identityA).isResultAccepted()); + assertSame(firstCallback, activeRequest.getSuccessCallback()); + } + + @Test + public void failedRetryIsIgnoredWhileARequestIsActive() { + IterableAuthRequestCoordinator.Request requestA = + coordinator.enqueue(identityA, null, false, true).getRequestToStart(); + + IterableAuthRequestCoordinator.EnqueueResult result = + coordinator.enqueue(identityB, null, true, true); + IterableAuthRequestCoordinator.Completion completion = + coordinator.complete(requestA, identityB); + + assertEquals( + IterableAuthRequestCoordinator.EnqueueStatus.IGNORED_FAILED_RETRY, + result.getStatus() + ); + assertNull(completion.getNextRequest()); + } + + @Test + public void newIdentityRunsAfterTheStaleRequestCompletes() { + IterableHelper.SuccessHandler callbackB = + mock(IterableHelper.SuccessHandler.class); + IterableAuthRequestCoordinator.Request requestA = + coordinator.enqueue(identityA, null, false, true).getRequestToStart(); + + IterableAuthRequestCoordinator.EnqueueResult enqueueB = + coordinator.enqueue(identityB, callbackB, false, true); + IterableAuthRequestCoordinator.Completion completion = + coordinator.complete(requestA, identityB); + + assertEquals( + IterableAuthRequestCoordinator.EnqueueStatus.QUEUED_FOR_NEW_IDENTITY, + enqueueB.getStatus() + ); + assertFalse(completion.isResultAccepted()); + assertSame(identityB, completion.getNextRequest().getIdentity()); + assertSame(callbackB, completion.getNextRequest().getSuccessCallback()); + assertFalse(completion.getNextRequest().hasFailedPriorAuth()); + } + + @Test + public void latestQueuedIdentityWins() { + Object identityC = new Object(); + IterableAuthRequestCoordinator.Request requestA = + coordinator.enqueue(identityA, null, false, true).getRequestToStart(); + coordinator.enqueue(identityB, null, false, true); + coordinator.enqueue(identityC, null, false, true); + + IterableAuthRequestCoordinator.Completion completion = + coordinator.complete(requestA, identityC); + + assertFalse(completion.isResultAccepted()); + assertSame(identityC, completion.getNextRequest().getIdentity()); + } + + @Test + public void clearingQueuedWorkLeavesNoSuccessor() { + IterableAuthRequestCoordinator.Request requestA = + coordinator.enqueue(identityA, null, false, true).getRequestToStart(); + coordinator.enqueue(identityB, null, false, true); + + coordinator.clearQueued(); + IterableAuthRequestCoordinator.Completion completion = + coordinator.complete(requestA, identityB); + + assertFalse(completion.isResultAccepted()); + assertNull(completion.getNextRequest()); + } + + @Test + public void orphanedCompletionCannotClearTheReplacement() { + IterableAuthRequestCoordinator.Request requestA = + coordinator.enqueue(identityA, null, false, true).getRequestToStart(); + coordinator.enqueue(identityB, null, false, true); + IterableAuthRequestCoordinator.Request requestB = + coordinator.complete(requestA, identityB).getNextRequest(); + + IterableAuthRequestCoordinator.Completion orphanedCompletion = + coordinator.complete(requestA, identityB); + + assertFalse(orphanedCompletion.isResultAccepted()); + assertNull(orphanedCompletion.getNextRequest()); + assertTrue(coordinator.isCurrent(requestB, identityB)); + } +} From 5638b42b85f66bd82d544ff5ff78b36080de77d8 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Thu, 20 Aug 2026 16:27:47 +0100 Subject: [PATCH 10/11] [SDK-547] Limit JWT fix to refresh ownership --- CHANGELOG.md | 6 +- checkstyle.xml | 5 - .../com/iterable/iterableapi/IterableApi.java | 42 +- .../iterableapi/IterableAuthDataRestorer.java | 232 --------- .../iterableapi/IterableAuthManager.java | 359 ++----------- .../IterableAuthRequestCoordinator.java | 154 ------ .../iterable/iterableapi/IterableKeychain.kt | 63 +-- .../iterableapi/IterableApiAuthTests.java | 89 ---- ...terableAuthDataRestoreIntegrationTest.java | 183 ------- .../IterableAuthDataRestorerTest.java | 135 ----- .../IterableAuthIdentityGuardTest.java | 305 ----------- .../IterableAuthRefreshOwnershipTest.java | 195 +++++-- .../IterableAuthRequestCoordinatorTest.java | 150 ------ .../IterableAuthTokenLifecycleTest.java | 491 ------------------ .../iterableapi/IterableKeychainTest.kt | 75 --- 15 files changed, 237 insertions(+), 2247 deletions(-) delete mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java delete mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index e0b471af7..23b7b1b79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,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. The pending refresh task is now the single source of ownership, and cancelled or replaced tasks cannot execute or clear their replacement. Refresh scheduling also records an explicit reason, such as token expiration, a 401 retry, or a missing stored token. -- 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 on the next launch. Crypto timeouts now preserve stored data and encryption state; timed-out operations are cancelled so they do not block later reads or writes. -- Fixed a JWT auth token belonging to a previous user being installed on the current session. A token fetched from `IterableAuthHandler.onAuthTokenRequested()`, or read back from encrypted storage, can arrive after the app has signed in a different user or called `initialize()` again; such a result is now discarded instead of stored. A login that happens *before* the handler is invoked still reuses the request already in flight, so no additional `onAuthTokenRequested()` call is introduced. -- Fixed `setEmail(email, authToken)` / `setUserId(userId, authToken)` replacing the refresh timer and clearing the invalid-token state when called repeatedly with an equal token value. A repeated login could postpone an expiration refresh indefinitely and mark a token ready that a 401 had just rejected. -- Fixed a timed-out stored-token read being mistaken for a confirmed missing JWT. Transient token-read timeouts are retried off the caller thread, and `IterableAuthHandler.onAuthTokenRequested()` is invoked only after a completed read confirms that the stored token is absent. If storage remains unavailable, JWT-required work stays blocked and restoration is retried when the app returns to the foreground. +- Fixed a race in JWT auth refresh scheduling that could leave overlapping timers active and repeatedly call `IterableAuthHandler.onAuthTokenRequested()`. Refresh scheduling now has a single task owner, rejects stale or duplicate tasks, and logs each schedule, skip, fire, cancellation, and error with its refresh reason. ## [3.10.0] ### Added diff --git a/checkstyle.xml b/checkstyle.xml index 3bbf7bcad..f0fada454 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -6,11 +6,6 @@ - - - - - diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index c4d43a546..fd7b1871a 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -22,7 +22,6 @@ import java.util.Objects; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ScheduledExecutorService; /** * Created by David Truong dt@iterable.com @@ -39,7 +38,7 @@ public class IterableApi { private String _email; private String _userId; String _userIdUnknown; - private volatile String _authToken; + private String _authToken; private boolean _debugMode; private Bundle _payloadData; private IterableNotificationData _notificationData; @@ -60,8 +59,9 @@ public class IterableApi { private String inboxSessionId; private IterableAuthManager authManager; private ConcurrentHashMap deviceAttributes = new ConcurrentHashMap<>(); - @VisibleForTesting IterableKeychain keychain; - @VisibleForTesting ScheduledExecutorService authDataRestoreExecutor; + private IterableKeychain keychain; + + //region Background Initialization - Delegated to IterableBackgroundInitializer //--------------------------------------------------------------------------------------- @@ -138,8 +138,9 @@ public String getAuthToken() { } private void checkAndUpdateAuthToken(@Nullable String authToken) { + // If authHandler exists and if authToken is new, it will be considered as a call to update the authToken. if (config.authHandler != null && authToken != null && authToken != _authToken) { - getAuthManager().useExplicitAuthToken(authToken); + setAuthToken(authToken); } } @@ -409,7 +410,7 @@ private void logoutPreviousUser() { embeddedManager.reset(); } if (authManager != null) { - authManager.resetForIdentityChange(); + authManager.reset(); } if (apiClient != null) { @@ -433,7 +434,7 @@ private void onLogin( getAuthManager().pauseAuthRetries(false); if (authToken != null) { - getAuthManager().useExplicitAuthToken(authToken); + setAuthToken(authToken); attemptMergeAndEventReplay(userIdOrEmail, isEmail, merge, replay, isUnknown, failureHandler); } else { getAuthManager().requestNewAuthToken(false, data -> attemptMergeAndEventReplay(userIdOrEmail, isEmail, merge, replay, isUnknown, failureHandler)); @@ -611,21 +612,24 @@ private void retrieveEmailAndUserId() { if (_applicationContext == null) { return; } - IterableKeychain iterableKeychain = getKeychain(); - if (iterableKeychain == null) { + if (iterableKeychain != null) { + _email = iterableKeychain.getEmail(); + _userId = iterableKeychain.getUserId(); + _userIdUnknown = iterableKeychain.getUserIdUnknown(); + _authToken = iterableKeychain.getAuthToken(); + } else { IterableLogger.e(TAG, "retrieveEmailAndUserId: Shared preference creation failed. Could not retrieve email/userId"); - return; } - _email = iterableKeychain.getEmail(); - _userId = iterableKeychain.getUserId(); - _userIdUnknown = iterableKeychain.getUserIdUnknown(); - if (config.authHandler == null || !checkSDKInitialization()) { - _authToken = iterableKeychain.getAuthToken(); - return; + if (config.authHandler != null && checkSDKInitialization()) { + if (_authToken != null) { + getAuthManager().queueExpirationRefresh(_authToken); + } else { + IterableLogger.d(TAG, "Auth token found as null. Rescheduling auth token refresh"); + getAuthManager().scheduleAuthTokenRefresh(authManager.getNextRetryInterval(), IterableAuthRefreshReason.STORED_TOKEN_MISSING, null); + } } - _authToken = getAuthManager().restoreAuthToken(iterableKeychain, authDataRestoreExecutor); } private class IterableApiAuthProvider implements IterableApiClient.AuthProvider { @@ -694,10 +698,6 @@ void setAuthToken(String authToken, boolean bypassAuth) { } } - void setRestoredAuthToken(@Nullable String authToken) { - _authToken = authToken; - } - protected void registerDeviceToken(final @Nullable String email, final @Nullable String userId, final @Nullable String authToken, final @NonNull String applicationName, final @NonNull String deviceToken, final Map deviceAttributes) { if (deviceToken != null) { if (!checkSDKInitialization() && _userIdUnknown == null) { diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java deleted file mode 100644 index af59369c9..000000000 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthDataRestorer.java +++ /dev/null @@ -1,232 +0,0 @@ -package com.iterable.iterableapi; - -import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; - -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; - -/** - * Retries a stored auth-token read without turning a transient timeout into a missing token. - */ -class IterableAuthDataRestorer { - private static final String TAG = "IterableAuthRestore"; - - @VisibleForTesting - static final int MAX_TIMEOUT_RETRIES = 2; - - @VisibleForTesting - static final long RETRY_DELAY_MS = 1000L; - - private static final ScheduledExecutorService RETRY_EXECUTOR = - Executors.newSingleThreadScheduledExecutor( - runnable -> { - Thread thread = new Thread(runnable, "IterableAuthRestore"); - thread.setDaemon(true); - return thread; - } - ); - - interface Callback { - void onAuthTokenRestored(@Nullable String authToken); - - void onAuthTokenUnavailable(); - } - - private final IterableKeychain keychain; - private final ScheduledExecutorService retryExecutor; - - @Nullable - private ScheduledFuture pendingRetry; - - @Nullable - private Callback callback; - - private int generation; - private boolean restoring; - private boolean unavailable; - - IterableAuthDataRestorer(IterableKeychain keychain) { - this(keychain, RETRY_EXECUTOR); - } - - @VisibleForTesting - IterableAuthDataRestorer( - IterableKeychain keychain, - ScheduledExecutorService retryExecutor - ) { - this.keychain = keychain; - this.retryExecutor = retryExecutor; - } - - void restore(KeychainReadResult initialRead, Callback callback) { - final int currentGeneration; - synchronized (this) { - cancelPendingRetry(); - generation++; - currentGeneration = generation; - this.callback = callback; - restoring = true; - unavailable = false; - } - - IterableLogger.d(TAG, "auth_restore action=start"); - handleRead(currentGeneration, 0, initialRead); - } - - /** - * Returns true while restoration is unresolved. An unavailable restore starts a new cycle; - * an already-running restore is left alone. - */ - synchronized boolean resumeIfUnresolved() { - if (restoring) { - IterableLogger.d( - TAG, - "auth_restore action=resume outcome=already_running source=foreground" - ); - return true; - } - if (!unavailable || callback == null) { - return false; - } - - generation++; - restoring = true; - unavailable = false; - IterableLogger.d( - TAG, - "auth_restore action=resume outcome=scheduled source=foreground" - ); - scheduleRead(generation, 0, 0); - return true; - } - - synchronized void cancel(String reason) { - boolean wasUnresolved = restoring || unavailable; - generation++; - cancelPendingRetry(); - callback = null; - restoring = false; - unavailable = false; - if (wasUnresolved) { - IterableLogger.d(TAG, "auth_restore action=cancel reason=" + reason); - } - } - - private void handleRead( - int currentGeneration, - int timeoutRetries, - KeychainReadResult result - ) { - if (!isCurrent(currentGeneration)) { - return; - } - - int attempt = timeoutRetries + 1; - if (result instanceof KeychainReadResult.Value) { - String authToken = ((KeychainReadResult.Value) result).getValue(); - complete(currentGeneration, authToken, attempt); - return; - } - - IterableLogger.d( - TAG, - "auth_restore action=read attempt=" - + attempt - + " outcome=timeout" - ); - if (timeoutRetries >= MAX_TIMEOUT_RETRIES) { - markUnavailable(currentGeneration, attempt); - } else { - scheduleRead(currentGeneration, timeoutRetries + 1, RETRY_DELAY_MS); - } - } - - private synchronized void scheduleRead( - int currentGeneration, - int timeoutRetries, - long delayMs - ) { - if (!isCurrent(currentGeneration)) { - return; - } - - pendingRetry = retryExecutor.schedule( - () -> { - synchronized (IterableAuthDataRestorer.this) { - if (!isCurrent(currentGeneration)) { - IterableLogger.d( - TAG, - "auth_restore action=ignore reason=stale_generation" - ); - return; - } - pendingRetry = null; - } - handleRead(currentGeneration, timeoutRetries, keychain.readAuthToken()); - }, - delayMs, - TimeUnit.MILLISECONDS - ); - } - - private synchronized void complete( - int currentGeneration, - @Nullable String authToken, - int attempt - ) { - if (!isCurrent(currentGeneration)) { - return; - } - - restoring = false; - unavailable = false; - pendingRetry = null; - IterableLogger.d( - TAG, - "auth_restore action=read attempt=" - + attempt - + " outcome=" - + (authToken == null ? "token_missing" : "token_found") - ); - - Callback currentCallback = callback; - callback = null; - if (currentCallback != null) { - // Keep completion ordered with cancel(): an explicit identity change must win. - currentCallback.onAuthTokenRestored(authToken); - } - } - - private synchronized void markUnavailable(int currentGeneration, int attempts) { - if (!isCurrent(currentGeneration)) { - return; - } - - restoring = false; - unavailable = true; - pendingRetry = null; - IterableLogger.w( - TAG, - "auth_restore action=complete attempts=" - + attempts - + " outcome=unavailable" - ); - if (callback != null) { - callback.onAuthTokenUnavailable(); - } - } - - private synchronized boolean isCurrent(int currentGeneration) { - return generation == currentGeneration && restoring; - } - - private void cancelPendingRetry() { - if (pendingRetry != null) { - pendingRetry.cancel(true); - pendingRetry = null; - } - } -} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index 0da973ec2..cfd053bc9 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -14,8 +14,6 @@ import java.util.TimerTask; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.atomic.AtomicReference; public class IterableAuthManager implements IterableActivityMonitor.AppStateCallback { private static final String TAG = "IterableAuth"; @@ -26,13 +24,11 @@ public class IterableAuthManager implements IterableActivityMonitor.AppStateCall * VALID: Last request succeeded with this token. * INVALID: A 401 JWT error was received; processing should pause. * UNKNOWN: A new token was obtained but not yet verified by a request. - * RESTORING: Startup auth is unresolved, so JWT-required work must wait. */ enum AuthState { VALID, INVALID, - UNKNOWN, - RESTORING + UNKNOWN } private enum RefreshCancellationReason { @@ -42,25 +38,6 @@ private enum RefreshCancellationReason { APP_BACKGROUNDED } - /** - * A snapshot of "who was signed in" when async auth work began. Auth results arrive from a - * developer callback and from encrypted storage, both of which can outlive the identity that - * asked for them; a result carrying a superseded identity must be dropped rather than stored. - * Comparison is by reference — {@code version} only exists to make log lines readable. - */ - private static final class AuthIdentity { - private final int version; - - AuthIdentity(int version) { - this.version = version; - } - - @Override - public String toString() { - return "identity#" + version; - } - } - /** * Listener interface for components that need to react when a new auth token is ready. */ @@ -78,12 +55,8 @@ interface AuthTokenReadyListener { volatile TimerTask scheduledRefreshTask; @VisibleForTesting volatile IterableAuthRefreshReason scheduledRefreshReason; - private final AtomicReference authDataRestorer = new AtomicReference<>(); - private final AtomicReference currentIdentity = - new AtomicReference<>(new AuthIdentity(0)); - private final IterableAuthRequestCoordinator authRequestCoordinator = - new IterableAuthRequestCoordinator<>(); private boolean hasFailedPriorAuth; + private boolean pendingAuth; private boolean requiresAuthRefresh; RetryPolicy authRetryPolicy; boolean pauseAuthRetry; @@ -94,8 +67,7 @@ interface AuthTokenReadyListener { private volatile AuthState authState = AuthState.UNKNOWN; private final ArrayList authTokenReadyListeners = new ArrayList<>(); - @VisibleForTesting - ExecutorService executor = Executors.newSingleThreadExecutor(); + private final ExecutorService executor = Executors.newSingleThreadExecutor(); IterableAuthManager(IterableApi api, IterableAuthHandler authHandler, RetryPolicy authRetryPolicy, long expiringAuthTokenRefreshPeriod) { this.api = api; @@ -123,11 +95,7 @@ boolean isAuthTokenReady() { if (authHandler == null) { return true; } - // A signed-out user has no token to restore, so RESTORING would block work indefinitely. - if (authState == AuthState.RESTORING && !hasIdentity()) { - return true; - } - return isReadyState(authState); + return authState != AuthState.INVALID; } /** @@ -142,136 +110,19 @@ AuthState getAuthState() { } /** - * Centralized auth state setter. Listeners are notified whenever auth moves from a blocked - * state to a ready state. + * Centralized auth state setter. Notifies AuthTokenReadyListeners only when + * transitioning from INVALID to a ready state (UNKNOWN or VALID), which means + * a new token has been obtained after a prior auth failure. */ private void setAuthState(AuthState newState) { AuthState previousState = this.authState; this.authState = newState; - if (!isReadyState(previousState) && isReadyState(newState)) { + if (previousState == AuthState.INVALID && newState != AuthState.INVALID) { notifyAuthTokenReadyListeners(); } } - private boolean isReadyState(AuthState state) { - return state == AuthState.VALID || state == AuthState.UNKNOWN; - } - - private boolean hasIdentity() { - return getEmailOrUserId() != null; - } - - /** - * Supersedes every auth result already in flight; returns the identity that replaces them. - * Only call this where a replacement token or refresh is about to be armed — superseding - * without a successor leaves auth with nothing pending. - */ - private AuthIdentity startNewIdentity() { - authRequestCoordinator.clearQueued(); - AuthIdentity previous; - AuthIdentity next; - do { - previous = currentIdentity.get(); - next = new AuthIdentity(previous.version + 1); - } while (!currentIdentity.compareAndSet(previous, next)); - return next; - } - - private boolean isStillCurrent(AuthIdentity identity) { - return currentIdentity.get() == identity; - } - - private void logSupersededRequest(AuthIdentity identity) { - IterableLogger.d( - TAG, - "auth_token action=discard started_as=" - + identity - + " now=" - + currentIdentity.get() - ); - } - - @Nullable - String restoreAuthToken( - IterableKeychain keychain, - @Nullable ScheduledExecutorService retryExecutor - ) { - KeychainReadResult initialRead = keychain.readAuthToken(); - setAuthState(AuthState.RESTORING); - final AuthIdentity identity = startNewIdentity(); - final IterableAuthDataRestorer restorer = retryExecutor == null - ? new IterableAuthDataRestorer(keychain) - : new IterableAuthDataRestorer(keychain, retryExecutor); - // Published before restore() because a successful initial read completes inline. - authDataRestorer.set(restorer); - restorer.restore( - initialRead, - new IterableAuthDataRestorer.Callback() { - @Override - public void onAuthTokenRestored(@Nullable String authToken) { - authDataRestorer.compareAndSet(restorer, null); - if (!isStillCurrent(identity)) { - IterableLogger.d( - TAG, - "auth_restore action=discard started_as=" - + identity - + " now=" - + currentIdentity.get() - ); - return; - } - api.setRestoredAuthToken(authToken); - handleRestoredAuthToken(authToken); - } - - @Override - public void onAuthTokenUnavailable() { - IterableLogger.w( - TAG, - "auth_restore action=block reason=storage_unavailable" - ); - } - } - ); - return initialRead.valueOrNull(); - } - - private void handleRestoredAuthToken(@Nullable String authToken) { - if (authToken != null) { - setAuthState(AuthState.UNKNOWN); - queueExpirationRefresh(authToken); - } else { - scheduleAuthTokenRefresh( - getNextRetryInterval(), - IterableAuthRefreshReason.STORED_TOKEN_MISSING, - null - ); - } - } - - void cancelAuthTokenRestore(String reason) { - IterableAuthDataRestorer restorer = authDataRestorer.getAndSet(null); - if (restorer != null) { - restorer.cancel(reason); - } - } - - void useExplicitAuthToken(String authToken) { - cancelAuthTokenRestore("explicit_token"); - boolean tokenChanged = authToken == null - ? api.getAuthToken() != null - : !authToken.equalsIgnoreCase(api.getAuthToken()); - api.setAuthToken(authToken); - // Re-arming on an unchanged token would postpone an expiry refresh indefinitely and - // downgrade INVALID to UNKNOWN, marking a token ready that a 401 just rejected. - if (authHandler != null && tokenChanged) { - startNewIdentity(); - setAuthState(AuthState.UNKNOWN); - queueExpirationRefresh(authToken); - } - } - private void notifyAuthTokenReadyListeners() { ArrayList listenersCopy = new ArrayList<>(authTokenReadyListeners); for (AuthTokenReadyListener listener : listenersCopy) { @@ -293,15 +144,6 @@ void reset() { setIsLastAuthTokenValid(false); } - void resetForIdentityChange() { - cancelAuthTokenRestore("identity_changed"); - startNewIdentity(); - if (authHandler != null) { - setAuthState(AuthState.RESTORING); - } - reset(); - } - void setIsLastAuthTokenValid(boolean isValid) { isLastAuthTokenValid = isValid; if (isValid) { @@ -332,146 +174,56 @@ public synchronized void requestNewAuthToken( } if (authHandler != null) { - if (this.hasFailedPriorAuth && hasFailedPriorAuth) { - return; - } - - IterableAuthRequestCoordinator.EnqueueResult enqueueResult = - authRequestCoordinator.enqueue( - currentIdentity.get(), - successCallback, - hasFailedPriorAuth, - shouldIgnoreRetryPolicy - ); - if (enqueueResult.getStatus() - == IterableAuthRequestCoordinator.EnqueueStatus.STARTED) { - submitAuthRequest(enqueueResult.getRequestToStart()); - } else if (enqueueResult.getStatus() - == IterableAuthRequestCoordinator.EnqueueStatus - .ALREADY_ACTIVE_FOR_IDENTITY && !hasFailedPriorAuth) { + if (!pendingAuth) { + if (!(this.hasFailedPriorAuth && hasFailedPriorAuth)) { + this.hasFailedPriorAuth = hasFailedPriorAuth; + pendingAuth = true; + + executor.submit(new Runnable() { + @Override + public void run() { + try { + if (isLastAuthTokenValid && !shouldIgnoreRetryPolicy) { + // if some JWT retry had valid token it will not fetch the auth token again from developer function + handleAuthTokenSuccess(IterableApi.getInstance().getAuthToken(), successCallback); + pendingAuth = false; + return; + } + + // Only request new auth token if app is in foreground + if (!isInForeground) { + IterableLogger.w(TAG, "Auth token request skipped - app is in background"); + pendingAuth = false; + return; + } + + final String authToken = authHandler.onAuthTokenRequested(); + pendingAuth = false; + retryCount++; + handleAuthTokenSuccess(authToken, successCallback); + } catch (final Exception e) { + retryCount++; + handleAuthTokenFailure(e); + } + } + }); + } + } else if (!hasFailedPriorAuth) { + //setFlag to resync auth after current auth returns requiresAuthRefresh = true; } - } else { - api.setAuthToken(null, true); - } - } - - private void submitAuthRequest( - IterableAuthRequestCoordinator.Request request - ) { - hasFailedPriorAuth = request.hasFailedPriorAuth(); - executor.submit(() -> executeAuthRequest(request)); - } - - private void executeAuthRequest( - IterableAuthRequestCoordinator.Request request - ) { - if (!authRequestCoordinator.isCurrent(request, currentIdentity.get())) { - finishSupersededRequest(request); - return; - } - - if (isLastAuthTokenValid && !request.shouldIgnoreRetryPolicy()) { - completeAuthRequestWithToken(request, api.getAuthToken()); - return; - } - - if (!isInForeground) { - IterableLogger.w(TAG, "Auth token request skipped - app is in background"); - IterableAuthRequestCoordinator.Completion completion = - authRequestCoordinator.complete(request, currentIdentity.get()); - if (!completion.isResultAccepted()) { - finishSupersededRequest(request, completion); - return; - } - submitNextAuthRequest(completion); - return; - } - - String authToken; - try { - authToken = authHandler.onAuthTokenRequested(); - retryCount++; - } catch (Exception e) { - retryCount++; - completeAuthRequestWithFailure(request, e); - return; - } - completeAuthRequestWithToken(request, authToken); - } - - private void completeAuthRequestWithToken( - IterableAuthRequestCoordinator.Request request, - String authToken - ) { - IterableAuthRequestCoordinator.Completion completion = - authRequestCoordinator.complete(request, currentIdentity.get()); - if (!completion.isResultAccepted()) { - finishSupersededRequest(request, completion); - return; - } - - try { - handleAuthTokenSuccess(authToken, request.getSuccessCallback()); - } catch (Exception e) { - retryCount++; - handleAuthTokenFailure(e); - } finally { - submitNextAuthRequest(completion); - } - } - private void completeAuthRequestWithFailure( - IterableAuthRequestCoordinator.Request request, - Exception exception - ) { - IterableAuthRequestCoordinator.Completion completion = - authRequestCoordinator.complete(request, currentIdentity.get()); - if (!completion.isResultAccepted()) { - finishSupersededRequest(request, completion); - return; - } - - try { - handleAuthTokenFailure(exception); - } finally { - submitNextAuthRequest(completion); - } - } - - private void finishSupersededRequest( - IterableAuthRequestCoordinator.Request request - ) { - IterableAuthRequestCoordinator.Completion completion = - authRequestCoordinator.complete(request, currentIdentity.get()); - finishSupersededRequest(request, completion); - } - - private void finishSupersededRequest( - IterableAuthRequestCoordinator.Request request, - IterableAuthRequestCoordinator.Completion completion - ) { - logSupersededRequest(request.getIdentity()); - reSyncAuth(); - submitNextAuthRequest(completion); - } - - private void submitNextAuthRequest( - IterableAuthRequestCoordinator.Completion completion - ) { - IterableAuthRequestCoordinator.Request nextRequest = - completion.getNextRequest(); - if (nextRequest != null) { - submitAuthRequest(nextRequest); + } else { + IterableApi.getInstance().setAuthToken(null, true); } } private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHandler successCallback) { if (authToken != null) { - // Token obtained but not yet verified by a request. Storing it before changing state - // ensures listeners cannot resume JWT work with the previous token. - api.setAuthToken(authToken); + // Token obtained but not yet verified by a request - set state to UNKNOWN. + // setAuthState will notify listeners only if previous state was INVALID. setAuthState(AuthState.UNKNOWN); + IterableApi.getInstance().setAuthToken(authToken); queueExpirationRefresh(authToken); if (successCallback != null) { @@ -479,7 +231,7 @@ private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHand } } else { handleAuthFailure(authToken, AuthFailureReason.AUTH_TOKEN_NULL); - api.setAuthToken(authToken); + IterableApi.getInstance().setAuthToken(authToken); scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.AUTH_HANDLER_RETRY, @@ -495,6 +247,7 @@ private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHand private void handleAuthTokenFailure(Throwable throwable) { IterableLogger.e(TAG, "Error while requesting Auth Token", throwable); handleAuthFailure(null, AuthFailureReason.AUTH_TOKEN_GENERATION_ERROR); + pendingAuth = false; scheduleAuthTokenRefresh( getNextRetryInterval(), IterableAuthRefreshReason.AUTH_HANDLER_RETRY, @@ -610,8 +363,8 @@ public void run() { } IterableLogger.d(TAG, "auth_refresh action=fire reason=" + reason); - if (hasIdentity()) { - requestNewAuthToken( + if (api.getEmail() != null || api.getUserId() != null) { + api.getAuthManager().requestNewAuthToken( false, successCallback, reason.ignoresRetryPolicy() @@ -716,7 +469,7 @@ private static String getJson(String strEncoded) throws UnsupportedEncodingExcep */ private void checkAndHandleAuthRefresh() { // First, check if current auth token needs refresh based on expiration - if (hasIdentity()) { + if (api.getEmail() != null || api.getUserId() != null) { String currentAuthToken = api.getAuthToken(); queueExpirationRefresh(currentAuthToken); } else { @@ -755,10 +508,6 @@ public void onSwitchToForeground() { try { IterableLogger.d(TAG, "App switched to foreground - enabling auth token requests"); isInForeground = true; - IterableAuthDataRestorer restorer = authDataRestorer.get(); - if (restorer != null && restorer.resumeIfUnresolved()) { - return; - } checkAndHandleAuthRefresh(); } catch (Exception e) { IterableLogger.e(TAG, "Error occurred in handling auth token refresh", e); diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java deleted file mode 100644 index d41321644..000000000 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthRequestCoordinator.java +++ /dev/null @@ -1,154 +0,0 @@ -package com.iterable.iterableapi; - -import androidx.annotation.Nullable; - -/** - * Serializes auth-token requests while keeping each callback bound to the identity that requested - * it. Identity comparison is by reference so a logout and login with the same value still creates - * a distinct auth lifecycle. - */ -class IterableAuthRequestCoordinator { - enum EnqueueStatus { - STARTED, - QUEUED_FOR_NEW_IDENTITY, - ALREADY_ACTIVE_FOR_IDENTITY, - IGNORED_FAILED_RETRY - } - - static final class Request { - private final I identity; - @Nullable - private final IterableHelper.SuccessHandler successCallback; - private final boolean hasFailedPriorAuth; - private final boolean shouldIgnoreRetryPolicy; - - Request( - I identity, - @Nullable IterableHelper.SuccessHandler successCallback, - boolean hasFailedPriorAuth, - boolean shouldIgnoreRetryPolicy - ) { - this.identity = identity; - this.successCallback = successCallback; - this.hasFailedPriorAuth = hasFailedPriorAuth; - this.shouldIgnoreRetryPolicy = shouldIgnoreRetryPolicy; - } - - I getIdentity() { - return identity; - } - - @Nullable - IterableHelper.SuccessHandler getSuccessCallback() { - return successCallback; - } - - boolean hasFailedPriorAuth() { - return hasFailedPriorAuth; - } - - boolean shouldIgnoreRetryPolicy() { - return shouldIgnoreRetryPolicy; - } - } - - static final class EnqueueResult { - private final EnqueueStatus status; - @Nullable - private final Request requestToStart; - - EnqueueResult(EnqueueStatus status, @Nullable Request requestToStart) { - this.status = status; - this.requestToStart = requestToStart; - } - - EnqueueStatus getStatus() { - return status; - } - - @Nullable - Request getRequestToStart() { - return requestToStart; - } - } - - static final class Completion { - private final boolean resultAccepted; - @Nullable - private final Request nextRequest; - - Completion(boolean resultAccepted, @Nullable Request nextRequest) { - this.resultAccepted = resultAccepted; - this.nextRequest = nextRequest; - } - - boolean isResultAccepted() { - return resultAccepted; - } - - @Nullable - Request getNextRequest() { - return nextRequest; - } - } - - @Nullable - private Request activeRequest; - @Nullable - private Request queuedRequest; - - synchronized EnqueueResult enqueue( - I identity, - @Nullable IterableHelper.SuccessHandler successCallback, - boolean hasFailedPriorAuth, - boolean shouldIgnoreRetryPolicy - ) { - Request request = new Request<>( - identity, - successCallback, - hasFailedPriorAuth, - shouldIgnoreRetryPolicy - ); - if (activeRequest == null) { - activeRequest = request; - return new EnqueueResult<>(EnqueueStatus.STARTED, request); - } - if (hasFailedPriorAuth) { - return new EnqueueResult<>(EnqueueStatus.IGNORED_FAILED_RETRY, null); - } - if (activeRequest.getIdentity() == identity) { - return new EnqueueResult<>( - EnqueueStatus.ALREADY_ACTIVE_FOR_IDENTITY, - null - ); - } - - queuedRequest = request; - return new EnqueueResult<>(EnqueueStatus.QUEUED_FOR_NEW_IDENTITY, null); - } - - synchronized boolean isCurrent(Request request, I currentIdentity) { - return activeRequest == request && request.getIdentity() == currentIdentity; - } - - synchronized Completion complete(Request request, I currentIdentity) { - if (activeRequest != request) { - return new Completion<>(false, null); - } - - boolean resultAccepted = request.getIdentity() == currentIdentity; - activeRequest = null; - - Request nextRequest = null; - if (queuedRequest != null && queuedRequest.getIdentity() == currentIdentity) { - nextRequest = queuedRequest; - activeRequest = nextRequest; - } - queuedRequest = null; - return new Completion<>(resultAccepted, nextRequest); - } - - synchronized void clearQueued() { - queuedRequest = null; - } -} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt index 67f58156e..7fff52fbc 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt @@ -2,20 +2,10 @@ package com.iterable.iterableapi import android.content.Context import android.content.SharedPreferences -import androidx.annotation.RestrictTo import java.util.concurrent.Callable import java.util.concurrent.Executors -import java.util.concurrent.TimeoutException import java.util.concurrent.TimeUnit -@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) -sealed interface KeychainReadResult { - data class Value(val value: String?) : KeychainReadResult - data object TimedOut : KeychainReadResult - - fun valueOrNull(): String? = (this as? Value)?.value -} - class IterableKeychain { companion object { private const val TAG = "IterableKeychain" @@ -26,7 +16,7 @@ class IterableKeychain { private const val PLAINTEXT_SUFFIX = "_plaintext" private const val CRYPTO_OPERATION_TIMEOUT_MS = 500L private const val KEY_ENCRYPTION_ENABLED = "iterable-encryption-enabled" - + private val cryptoExecutor = Executors.newSingleThreadExecutor() } @@ -81,15 +71,7 @@ class IterableKeychain { } private fun runWithTimeout(callable: Callable): T { - 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 - } + return cryptoExecutor.submit(callable).get(CRYPTO_OPERATION_TIMEOUT_MS, TimeUnit.MILLISECONDS) } private fun handleDecryptionError(e: Exception? = null) { @@ -123,35 +105,24 @@ class IterableKeychain { } } - private fun secureGet(key: String): String? = - when (val result = readValue(key)) { - is KeychainReadResult.Value -> result.value - KeychainReadResult.TimedOut -> null - } - - private fun readValue(key: String): KeychainReadResult { + private fun secureGet(key: String): String? { val hasPlainText = sharedPrefs.getBoolean(key + PLAINTEXT_SUFFIX, false) if (!encryption) { - val value = if (hasPlainText) sharedPrefs.getString(key, null) else null - return KeychainReadResult.Value(value) + if (hasPlainText) { + return sharedPrefs.getString(key, null) + } else { + return null + } } else if (hasPlainText) { - return KeychainReadResult.Value(sharedPrefs.getString(key, null)) + return sharedPrefs.getString(key, null) } - val encryptedValue = sharedPrefs.getString(key, null) - ?: return KeychainReadResult.Value(null) + val encryptedValue = sharedPrefs.getString(key, null) ?: return null return try { - KeychainReadResult.Value(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. Keep the timeout - // distinct so auth restoration cannot mistake it for a missing value. (SDK-547) - IterableLogger.w(TAG, "Crypto operation timed out; keeping encrypted data for retry.") - KeychainReadResult.TimedOut + encryptor?.let { runWithTimeout { it.decrypt(encryptedValue) } } } catch (e: Exception) { handleDecryptionError(e) - KeychainReadResult.Value(null) + null } } @@ -174,14 +145,6 @@ 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) @@ -197,8 +160,6 @@ class IterableKeychain { fun saveUserId(userId: String?) = secureSave(KEY_USER_ID, userId) fun getAuthToken() = secureGet(KEY_AUTH_TOKEN) - @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) - fun readAuthToken() = readValue(KEY_AUTH_TOKEN) fun saveAuthToken(authToken: String?) = secureSave(KEY_AUTH_TOKEN, authToken) fun getUserIdUnknown() = secureGet(KEY_UNKNOWN_USER_ID) diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java index af92eed85..ebc879508 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java @@ -11,10 +11,7 @@ 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; @@ -26,11 +23,8 @@ 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; @@ -515,87 +509,4 @@ 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: concurrent callers (foreground refresh, 401 retry, an already-firing timer) must all - // contend for one scheduler-owned task. The pending task, rather than a separate boolean, is the - // source of truth. - // - // We drive scheduleAuthTokenRefresh directly rather than requestNewAuthToken: the executor is - // not injectable (see @Ignore'd tests above) and its auth request coordinator 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 to maximize contention. - 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, - IterableAuthRefreshReason.TOKEN_EXPIRING, - 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()); - } - } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java deleted file mode 100644 index 285828721..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestoreIntegrationTest.java +++ /dev/null @@ -1,183 +0,0 @@ -package com.iterable.iterableapi; - -import org.junit.Before; -import org.junit.Test; - -import java.util.ArrayDeque; -import java.util.Queue; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.eq; -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.mockito.Mockito.when; - -public class IterableAuthDataRestoreIntegrationTest extends BaseTest { - private static final String EMAIL = "user@example.com"; - private static final String NEW_EMAIL = "new@example.com"; - private static final String VALID_JWT = - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." - + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9." - + "mYtgSqdUIxK8_RnYBTUP4cmpKw83aKi7cMiixF3qMB4"; - - private IterableAuthHandler authHandler; - private IterableKeychain keychain; - private ScheduledExecutorService retryExecutor; - private Queue scheduledTasks; - - @Before - public void setUp() { - IterableApi.sharedInstance = new IterableApi(); - authHandler = mock(IterableAuthHandler.class); - keychain = mock(IterableKeychain.class); - doReturn(EMAIL).when(keychain).getEmail(); - - retryExecutor = mock(ScheduledExecutorService.class); - scheduledTasks = new ArrayDeque<>(); - when( - retryExecutor.schedule( - any(Runnable.class), - anyLong(), - eq(TimeUnit.MILLISECONDS) - ) - ).thenAnswer( - invocation -> { - scheduledTasks.add(invocation.getArgument(0)); - return mock(ScheduledFuture.class); - } - ); - - IterableApi.sharedInstance.keychain = keychain; - IterableApi.sharedInstance.authDataRestoreExecutor = retryExecutor; - } - - @Test - public void repeatedTimeoutsNeverRequestANewTokenWithoutConfirmedAbsence() { - doReturn(KeychainReadResult.TimedOut.INSTANCE) - .when(keychain) - .readAuthToken(); - - initialize(); - runNext(); - runNext(); - - IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); - assertEquals(IterableAuthManager.AuthState.RESTORING, authManager.getAuthState()); - assertFalse(authManager.isAuthTokenReady()); - verify(authHandler, never()).onAuthTokenRequested(); - - authManager.onSwitchToForeground(); - - assertEquals(1, scheduledTasks.size()); - verify(authHandler, never()).onAuthTokenRequested(); - } - - @Test - public void timeoutThenSuccessRestoresTokenWithoutCallingClientHandler() { - doReturn( - KeychainReadResult.TimedOut.INSTANCE, - new KeychainReadResult.Value(VALID_JWT) - ) - .when(keychain) - .readAuthToken(); - - initialize(); - runNext(); - - IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); - assertEquals(EMAIL, IterableApi.getInstance().getEmail()); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - assertTrue(authManager.isAuthTokenReady()); - assertEquals( - IterableAuthRefreshReason.TOKEN_EXPIRING, - authManager.scheduledRefreshReason - ); - verify(authHandler, never()).onAuthTokenRequested(); - } - - @Test - public void confirmedMissingTokenSchedulesOneReasonedRefresh() { - doReturn(new KeychainReadResult.Value(null)) - .when(keychain) - .readAuthToken(); - - initialize(); - - IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); - assertEquals(IterableAuthManager.AuthState.RESTORING, authManager.getAuthState()); - assertFalse(authManager.isAuthTokenReady()); - assertEquals( - IterableAuthRefreshReason.STORED_TOKEN_MISSING, - authManager.scheduledRefreshReason - ); - verify(authHandler, never()).onAuthTokenRequested(); - } - - @Test - public void explicitTokenAfterConfirmedAbsenceUnblocksAuth() { - doReturn(new KeychainReadResult.Value(null)) - .when(keychain) - .readAuthToken(); - - initialize(); - IterableApi.getInstance().setEmail(EMAIL, VALID_JWT); - - IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - assertTrue(authManager.isAuthTokenReady()); - assertEquals( - IterableAuthRefreshReason.TOKEN_EXPIRING, - authManager.scheduledRefreshReason - ); - verify(authHandler, never()).onAuthTokenRequested(); - } - - @Test - public void explicitLoginWinsOverAQueuedRestoreRetry() { - doReturn( - KeychainReadResult.TimedOut.INSTANCE, - new KeychainReadResult.Value("old-token") - ) - .when(keychain) - .readAuthToken(); - - initialize(); - IterableAuthManager authManager = IterableApi.getInstance().getAuthManager(); - AtomicReference tokenWhenAuthBecameReady = new AtomicReference<>(); - authManager.addAuthTokenReadyListener( - () -> tokenWhenAuthBecameReady.set(IterableApi.getInstance().getAuthToken()) - ); - - IterableApi.getInstance().setEmail(NEW_EMAIL, VALID_JWT); - runNext(); - - assertEquals(NEW_EMAIL, IterableApi.getInstance().getEmail()); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - assertEquals(VALID_JWT, tokenWhenAuthBecameReady.get()); - } - - private void initialize() { - IterableApi.initialize( - getContext(), - "apiKey", - new IterableConfig.Builder() - .setAutoPushRegistration(false) - .setAuthHandler(authHandler) - .build() - ); - } - - private void runNext() { - scheduledTasks.remove().run(); - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java deleted file mode 100644 index 78f029dfd..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthDataRestorerTest.java +++ /dev/null @@ -1,135 +0,0 @@ -package com.iterable.iterableapi; - -import org.junit.Before; -import org.junit.Test; - -import java.util.ArrayDeque; -import java.util.Queue; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class IterableAuthDataRestorerTest { - private IterableKeychain keychain; - private ScheduledExecutorService retryExecutor; - private Queue scheduledTasks; - private RecordingCallback callback; - private IterableAuthDataRestorer restorer; - - @Before - public void setUp() { - keychain = mock(IterableKeychain.class); - retryExecutor = mock(ScheduledExecutorService.class); - scheduledTasks = new ArrayDeque<>(); - when( - retryExecutor.schedule( - any(Runnable.class), - anyLong(), - eq(TimeUnit.MILLISECONDS) - ) - ).thenAnswer( - invocation -> { - scheduledTasks.add(invocation.getArgument(0)); - return mock(ScheduledFuture.class); - } - ); - - callback = new RecordingCallback(); - restorer = new IterableAuthDataRestorer(keychain, retryExecutor); - } - - @Test - public void timeoutThenSuccessfulReadRestoresToken() { - doReturn(new KeychainReadResult.Value("stored-token")) - .when(keychain) - .readAuthToken(); - - restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); - runNext(); - - assertEquals("stored-token", callback.restoredToken); - assertEquals(0, callback.unavailableCount); - } - - @Test - public void exhaustingTimeoutRetriesRemainsUnavailable() { - doReturn(KeychainReadResult.TimedOut.INSTANCE) - .when(keychain) - .readAuthToken(); - - restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); - runNext(); - runNext(); - - verify(keychain, times(IterableAuthDataRestorer.MAX_TIMEOUT_RETRIES)) - .readAuthToken(); - assertNull(callback.restoredToken); - assertEquals(1, callback.unavailableCount); - assertTrue(restorer.resumeIfUnresolved()); - assertEquals(1, scheduledTasks.size()); - } - - @Test - public void foregroundRetryCanRecoverAfterAnUnavailableCycle() { - doReturn( - KeychainReadResult.TimedOut.INSTANCE, - KeychainReadResult.TimedOut.INSTANCE, - new KeychainReadResult.Value("stored-token") - ) - .when(keychain) - .readAuthToken(); - - restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); - runNext(); - runNext(); - assertEquals(1, callback.unavailableCount); - - assertTrue(restorer.resumeIfUnresolved()); - runNext(); - - assertEquals("stored-token", callback.restoredToken); - } - - @Test - public void cancellationPreventsAQueuedRetryFromRestoringAStaleToken() { - restorer.restore(KeychainReadResult.TimedOut.INSTANCE, callback); - restorer.cancel("identity_changed"); - runNext(); - - assertNull(callback.restoredToken); - assertEquals(0, callback.unavailableCount); - verify(keychain, never()).readAuthToken(); - } - - private void runNext() { - scheduledTasks.remove().run(); - } - - private static class RecordingCallback implements IterableAuthDataRestorer.Callback { - String restoredToken; - int unavailableCount; - - @Override - public void onAuthTokenRestored(String authToken) { - restoredToken = authToken; - } - - @Override - public void onAuthTokenUnavailable() { - unavailableCount++; - } - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java deleted file mode 100644 index c3f394b74..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthIdentityGuardTest.java +++ /dev/null @@ -1,305 +0,0 @@ -package com.iterable.iterableapi; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.List; -import java.util.Queue; -import java.util.Timer; -import java.util.TimerTask; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Future; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -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.mockito.Mockito.when; - -/** - * Auth results arrive asynchronously from a developer callback and from encrypted storage. Both can - * outlive the identity that asked for them, and storing a superseded result puts one user's token on - * another user's session. - */ -public class IterableAuthIdentityGuardTest extends BaseTest { - private static final String JWT_HEADER_AND_PAYLOAD = - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." - + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9."; - // Only the payload segment is parsed, so a distinct signature is enough to make a distinct token. - private static final String TOKEN_A = JWT_HEADER_AND_PAYLOAD + "signature-for-user-a"; - private static final String TOKEN_B = JWT_HEADER_AND_PAYLOAD + "signature-for-user-b"; - - private IterableApi api; - private IterableAuthHandler authHandler; - private IterableAuthManager authManager; - private IterableKeychain keychain; - private ScheduledExecutorService retryExecutor; - private Queue restoreRetries; - private List submittedAuthWork; - - @Before - public void setUp() { - api = mock(IterableApi.class); - when(api.getEmail()).thenReturn("user-a@example.com"); - authHandler = mock(IterableAuthHandler.class); - - authManager = new IterableAuthManager( - api, - authHandler, - new RetryPolicy(3, 1, RetryPolicy.Type.LINEAR), - 60_000 - ); - authManager.timer = new RetainingTimer(); - - submittedAuthWork = new ArrayList<>(); - ExecutorService executor = mock(ExecutorService.class); - when(executor.submit(any(Runnable.class))).thenAnswer( - invocation -> { - submittedAuthWork.add(invocation.getArgument(0)); - return mock(Future.class); - } - ); - authManager.executor = executor; - - keychain = mock(IterableKeychain.class); - retryExecutor = mock(ScheduledExecutorService.class); - restoreRetries = new ArrayDeque<>(); - when( - retryExecutor.schedule( - any(Runnable.class), - anyLong(), - eq(TimeUnit.MILLISECONDS) - ) - ).thenAnswer( - invocation -> { - restoreRetries.add(invocation.getArgument(0)); - return mock(ScheduledFuture.class); - } - ); - } - - @After - public void tearDown() { - authManager.clearRefreshTimer(); - } - - @Test - public void anOrphanedRestorerCannotStoreItsTokenAfterANewRestoreBegins() { - doReturn(KeychainReadResult.TimedOut.INSTANCE).when(keychain).readAuthToken(); - authManager.restoreAuthToken(keychain, retryExecutor); - assertEquals(1, restoreRetries.size()); - - doReturn(new KeychainReadResult.Value(TOKEN_B)).when(keychain).readAuthToken(); - authManager.restoreAuthToken(keychain, retryExecutor); - verify(api).setRestoredAuthToken(TOKEN_B); - - doReturn(new KeychainReadResult.Value(TOKEN_A)).when(keychain).readAuthToken(); - restoreRetries.remove().run(); - - verify(api, never()).setRestoredAuthToken(TOKEN_A); - } - - /** - * {@code onAuthTokenRequested} is a blocking developer callback, so the user can change while it - * runs. The token it returns belongs to the user who was signed in when it was called. - */ - @Test - public void aHandlerResultIsDiscardedWhenTheIdentityChangesWhileTheHandlerRuns() { - when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { - authManager.resetForIdentityChange(); - when(api.getEmail()).thenReturn("user-b@example.com"); - return TOKEN_A; - }); - authManager.requestNewAuthToken(false, null); - assertEquals(1, submittedAuthWork.size()); - - submittedAuthWork.get(0).run(); - - verify(api, never()).setAuthToken(TOKEN_A); - verify(authHandler, never()).onTokenRegistrationSuccessful(anyString()); - } - - /** - * A request that arrives while this one is in flight is deferred rather than started, and only - * a stored result replays it. Discarding must replay it too, or the new identity is left with no - * token and nothing scheduled. - */ - @Test - public void discardingAStaleResultStillHonoursTheRefreshDeferredBehindIt() { - when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { - authManager.requestNewAuthToken(false, null); - authManager.resetForIdentityChange(); - when(api.getEmail()).thenReturn("user-b@example.com"); - return TOKEN_A; - }); - authManager.requestNewAuthToken(false, null); - - submittedAuthWork.get(0).run(); - - verify(api, never()).setAuthToken(TOKEN_A); - assertNotNull( - "the deferred refresh must be armed for the identity that replaced this one", - authManager.scheduledRefreshTask - ); - } - - /** A queued login must keep its own callback when it replaces a superseded request. */ - @Test - public void anIdentityChangeBeforeTheHandlerRunsUsesTheNewIdentityCallback() { - IterableHelper.SuccessHandler callbackA = - mock(IterableHelper.SuccessHandler.class); - IterableHelper.SuccessHandler callbackB = - mock(IterableHelper.SuccessHandler.class); - when(authHandler.onAuthTokenRequested()).thenReturn(TOKEN_B); - authManager.requestNewAuthToken(false, callbackA); - - authManager.resetForIdentityChange(); - when(api.getEmail()).thenReturn("user-b@example.com"); - authManager.requestNewAuthToken(false, callbackB); - assertEquals(1, submittedAuthWork.size()); - - submittedAuthWork.get(0).run(); - assertEquals(2, submittedAuthWork.size()); - submittedAuthWork.get(1).run(); - - verify(api).setAuthToken(TOKEN_B); - verify(callbackA, never()).onSuccess(any()); - verify(callbackB).onSuccess(any()); - } - - /** - * A discarded result must still release coordinator ownership, or every later request is - * refused for the life of the process. - */ - @Test - public void discardingAStaleFailureLeavesAuthAbleToRequestAgain() { - when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { - authManager.resetForIdentityChange(); - throw new RuntimeException("boom"); - }); - authManager.requestNewAuthToken(false, null); - submittedAuthWork.get(0).run(); - - authManager.requestNewAuthToken(false, null); - - assertEquals(2, submittedAuthWork.size()); - } - - /** - * An orphaned restorer completing must not clear the field out from under the restore that - * replaced it — doing so loses the foreground retry that recovers an unresolved read. - */ - @Test - public void anOrphanedRestorerDoesNotEvictTheRestoreThatReplacedIt() { - doReturn(KeychainReadResult.TimedOut.INSTANCE).when(keychain).readAuthToken(); - authManager.restoreAuthToken(keychain, retryExecutor); - authManager.restoreAuthToken(keychain, retryExecutor); - assertEquals(2, restoreRetries.size()); - - doReturn(new KeychainReadResult.Value(TOKEN_A)).when(keychain).readAuthToken(); - restoreRetries.remove().run(); - - authManager.authRetryPolicy = new RetryPolicy(3, 60_000, RetryPolicy.Type.LINEAR); - authManager.onSwitchToForeground(); - - assertNull(authManager.scheduledRefreshTask); - } - - @Test - public void aHandlerFailureForASupersededIdentityIsNotReported() { - when(authHandler.onAuthTokenRequested()).thenAnswer(invocation -> { - authManager.resetForIdentityChange(); - throw new RuntimeException("boom"); - }); - authManager.requestNewAuthToken(false, null); - - submittedAuthWork.get(0).run(); - - verify(authHandler, never()).onAuthFailure(any(AuthFailure.class)); - } - - @Test - public void anInFlightHandlerResultStillLandsWhenTheIdentityIsUnchanged() { - when(authHandler.onAuthTokenRequested()).thenReturn(TOKEN_A); - authManager.requestNewAuthToken(false, null); - - submittedAuthWork.get(0).run(); - - verify(api).setAuthToken(TOKEN_A); - verify(authHandler).onTokenRegistrationSuccessful(TOKEN_A); - } - - @Test - public void repeatingAnEqualExplicitTokenDoesNotRearmOrUnblockAnInvalidToken() { - when(api.getAuthToken()).thenReturn(TOKEN_A); - authManager.setAuthTokenInvalid(); - - authManager.useExplicitAuthToken(new String(TOKEN_A)); - - assertEquals(IterableAuthManager.AuthState.INVALID, authManager.getAuthState()); - assertNull(authManager.scheduledRefreshTask); - } - - @Test - public void aGenuinelyNewExplicitTokenStillArmsARefresh() { - when(api.getAuthToken()).thenReturn(TOKEN_A); - authManager.setAuthTokenInvalid(); - - authManager.useExplicitAuthToken(TOKEN_B); - - assertEquals(IterableAuthManager.AuthState.UNKNOWN, authManager.getAuthState()); - assertNotNull(authManager.scheduledRefreshTask); - } - - @Test - public void restoringWithoutAnIdentityDoesNotBlockWork() { - when(api.getEmail()).thenReturn(null); - when(api.getUserId()).thenReturn(null); - - authManager.resetForIdentityChange(); - - assertEquals(IterableAuthManager.AuthState.RESTORING, authManager.getAuthState()); - assertTrue(authManager.isAuthTokenReady()); - } - - @Test - public void restoringWithAnIdentityStillBlocksWork() { - when(api.getEmail()).thenReturn("user-b@example.com"); - - authManager.resetForIdentityChange(); - - assertFalse(authManager.isAuthTokenReady()); - } - - private static class RetainingTimer extends Timer { - RetainingTimer() { - super(true); - super.cancel(); - } - - @Override - public void schedule(TimerTask task, long delay) { - // Retain nothing; tests only assert on the manager's ownership field. - } - - @Override - public void cancel() { - // Keep the timer usable across a clearRefreshTimer() call. - } - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java index 46908dd48..6cc5e6320 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java @@ -5,46 +5,34 @@ import org.junit.Test; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static org.mockito.ArgumentMatchers.any; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class IterableAuthRefreshOwnershipTest extends BaseTest { - private static final String VALID_JWT = - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." - + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9." - + "mYtgSqdUIxK8_RnYBTUP4cmpKw83aKi7cMiixF3qMB4"; - private IterableApi api; - private IterableAuthManager authManager; - private IterableAuthHandler authHandler; - private ExecutorService executor; + private RecordingAuthManager authManager; @Before public void setUp() { api = mock(IterableApi.class); when(api.getEmail()).thenReturn("user@example.com"); - authHandler = mock(IterableAuthHandler.class); - - authManager = new IterableAuthManager( - api, - authHandler, - new RetryPolicy(3, 1, RetryPolicy.Type.LINEAR), - 60_000 - ); - executor = mock(ExecutorService.class); - authManager.executor = executor; + authManager = new RecordingAuthManager(api); + when(api.getAuthManager()).thenReturn(authManager); } @After @@ -52,6 +40,46 @@ public void tearDown() { authManager.clearRefreshTimer(); } + @Test + public void concurrentSchedulingCreatesOneRefreshTask() throws Exception { + RetainingTimer timer = new RetainingTimer(); + authManager.timer = timer; + int threadCount = 8; + CyclicBarrier barrier = new CyclicBarrier(threadCount); + ExecutorService callers = Executors.newFixedThreadPool(threadCount); + List> futures = new ArrayList<>(); + + try { + for (int i = 0; i < threadCount; i++) { + futures.add( + callers.submit( + () -> { + barrier.await(); + authManager.scheduleAuthTokenRefresh( + 60_000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null + ); + return null; + } + ) + ); + } + for (Future future : futures) { + future.get(); + } + } finally { + callers.shutdownNow(); + } + + assertEquals(1, timer.taskCount()); + assertSame(timer.lastTask(), authManager.scheduledRefreshTask); + assertEquals( + IterableAuthRefreshReason.TOKEN_EXPIRING, + authManager.scheduledRefreshReason + ); + } + @Test public void staleTaskCannotRunOrClearItsReplacement() { RetainingTimer firstTimer = new RetainingTimer(); @@ -76,62 +104,121 @@ public void staleTaskCannotRunOrClearItsReplacement() { staleTask.run(); assertSame(replacementTask, authManager.scheduledRefreshTask); - assertEquals( - IterableAuthRefreshReason.TOKEN_EXPIRING, - authManager.scheduledRefreshReason - ); - verify(executor, never()).submit(any(Runnable.class)); + assertEquals(0, authManager.requestCount.get()); replacementTask.run(); assertNull(authManager.scheduledRefreshTask); assertNull(authManager.scheduledRefreshReason); - verify(executor).submit(any(Runnable.class)); + assertEquals(1, authManager.requestCount.get()); } @Test - public void firingTaskReleasesOwnershipBeforeAnotherRefreshIsScheduled() { + public void duplicateScheduleKeepsOriginalCallbackAndPolicy() { RetainingTimer timer = new RetainingTimer(); authManager.timer = timer; + IterableHelper.SuccessHandler firstCallback = + mock(IterableHelper.SuccessHandler.class); + IterableHelper.SuccessHandler secondCallback = + mock(IterableHelper.SuccessHandler.class); + authManager.scheduleAuthTokenRefresh( 1000, + IterableAuthRefreshReason.JWT_401, + firstCallback + ); + authManager.scheduleAuthTokenRefresh( + 2000, IterableAuthRefreshReason.TOKEN_EXPIRING, - null + secondCallback ); timer.lastTask().run(); + + assertEquals(1, timer.taskCount()); + assertSame(firstCallback, authManager.lastSuccessCallback); + assertFalse(authManager.lastIgnoreRetryPolicy); + } + + @Test + public void schedulingFailureReleasesOwnership() { + authManager.timer = new FailingTimer(); + + authManager.scheduleAuthTokenRefresh( + 1000, + IterableAuthRefreshReason.JWT_401, + null + ); + + assertNull(authManager.timer); assertNull(authManager.scheduledRefreshTask); + assertNull(authManager.scheduledRefreshReason); + RetainingTimer replacementTimer = new RetainingTimer(); + authManager.timer = replacementTimer; authManager.scheduleAuthTokenRefresh( 2000, - IterableAuthRefreshReason.TOKEN_EXPIRING, + IterableAuthRefreshReason.JWT_401, null ); - assertEquals(2, timer.taskCount()); - assertSame(timer.lastTask(), authManager.scheduledRefreshTask); + assertEquals(1, replacementTimer.taskCount()); } @Test - public void generatedTokenIsStoredOnTheManagersApiInstance() { - IterableApi replacementSharedInstance = mock(IterableApi.class); - IterableApi.sharedInstance = replacementSharedInstance; - when(authHandler.onAuthTokenRequested()).thenReturn(VALID_JWT); - when(executor.submit(any(Runnable.class))).thenAnswer( - invocation -> { - invocation.getArgument(0).run(); - return mock(Future.class); - } + public void scheduledLifecycleRefreshStillIgnoresPausedRetries() { + RetainingTimer timer = new RetainingTimer(); + authManager.timer = timer; + authManager.pauseAuthRetries(true); + + authManager.scheduleAuthTokenRefresh( + 1000, + IterableAuthRefreshReason.JWT_401, + null ); + assertEquals(0, timer.taskCount()); - authManager.requestNewAuthToken(false, null); + authManager.scheduleAuthTokenRefresh( + 1000, + IterableAuthRefreshReason.TOKEN_EXPIRING, + null + ); - verify(api).setAuthToken(VALID_JWT); - verify(replacementSharedInstance, never()).setAuthToken(VALID_JWT); + assertEquals(1, timer.taskCount()); + assertTrue( + IterableAuthRefreshReason.TOKEN_EXPIRING.ignoresRetryPolicy() + ); + } + + private static class RecordingAuthManager extends IterableAuthManager { + private final AtomicInteger requestCount = new AtomicInteger(); + private IterableHelper.SuccessHandler lastSuccessCallback; + private boolean lastIgnoreRetryPolicy; + + RecordingAuthManager(IterableApi api) { + super( + api, + mock(IterableAuthHandler.class), + new RetryPolicy(3, 1, RetryPolicy.Type.LINEAR), + 60_000 + ); + } + + @Override + public synchronized void requestNewAuthToken( + boolean hasFailedPriorAuth, + IterableHelper.SuccessHandler successCallback, + boolean shouldIgnoreRetryPolicy + ) { + requestCount.incrementAndGet(); + lastSuccessCallback = successCallback; + lastIgnoreRetryPolicy = shouldIgnoreRetryPolicy; + } } private static class RetainingTimer extends Timer { - private final List tasks = new ArrayList<>(); + private final List tasks = + Collections.synchronizedList(new ArrayList<>()); RetainingTimer() { super(true); @@ -141,11 +228,16 @@ private static class RetainingTimer extends Timer { @Override public void schedule(TimerTask task, long delay) { tasks.add(task); + try { + Thread.sleep(25); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } @Override public void cancel() { - // Retain tasks so a test can run a task after cancellation. + // Keep tasks available so stale-task behavior can be tested deterministically. } TimerTask lastTask() { @@ -156,4 +248,15 @@ int taskCount() { return tasks.size(); } } + + private static class FailingTimer extends Timer { + FailingTimer() { + super(true); + } + + @Override + public void schedule(TimerTask task, long delay) { + throw new IllegalStateException("timer rejected task"); + } + } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java deleted file mode 100644 index cf9397a84..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRequestCoordinatorTest.java +++ /dev/null @@ -1,150 +0,0 @@ -package com.iterable.iterableapi; - -import org.junit.Before; -import org.junit.Test; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; - -public class IterableAuthRequestCoordinatorTest { - private IterableAuthRequestCoordinator coordinator; - private Object identityA; - private Object identityB; - - @Before - public void setUp() { - coordinator = new IterableAuthRequestCoordinator<>(); - identityA = new Object(); - identityB = new Object(); - } - - @Test - public void firstRequestStartsImmediately() { - IterableHelper.SuccessHandler callback = mock(IterableHelper.SuccessHandler.class); - - IterableAuthRequestCoordinator.EnqueueResult result = - coordinator.enqueue(identityA, callback, false, true); - - assertEquals( - IterableAuthRequestCoordinator.EnqueueStatus.STARTED, - result.getStatus() - ); - assertSame(callback, result.getRequestToStart().getSuccessCallback()); - assertFalse(result.getRequestToStart().hasFailedPriorAuth()); - assertTrue(result.getRequestToStart().shouldIgnoreRetryPolicy()); - } - - @Test - public void sameIdentityKeepsTheActiveRequest() { - IterableHelper.SuccessHandler firstCallback = - mock(IterableHelper.SuccessHandler.class); - IterableHelper.SuccessHandler secondCallback = - mock(IterableHelper.SuccessHandler.class); - IterableAuthRequestCoordinator.Request activeRequest = - coordinator.enqueue( - identityA, - firstCallback, - false, - true - ).getRequestToStart(); - - IterableAuthRequestCoordinator.EnqueueResult result = - coordinator.enqueue(identityA, secondCallback, false, false); - - assertEquals( - IterableAuthRequestCoordinator.EnqueueStatus.ALREADY_ACTIVE_FOR_IDENTITY, - result.getStatus() - ); - assertNull(result.getRequestToStart()); - assertTrue(coordinator.complete(activeRequest, identityA).isResultAccepted()); - assertSame(firstCallback, activeRequest.getSuccessCallback()); - } - - @Test - public void failedRetryIsIgnoredWhileARequestIsActive() { - IterableAuthRequestCoordinator.Request requestA = - coordinator.enqueue(identityA, null, false, true).getRequestToStart(); - - IterableAuthRequestCoordinator.EnqueueResult result = - coordinator.enqueue(identityB, null, true, true); - IterableAuthRequestCoordinator.Completion completion = - coordinator.complete(requestA, identityB); - - assertEquals( - IterableAuthRequestCoordinator.EnqueueStatus.IGNORED_FAILED_RETRY, - result.getStatus() - ); - assertNull(completion.getNextRequest()); - } - - @Test - public void newIdentityRunsAfterTheStaleRequestCompletes() { - IterableHelper.SuccessHandler callbackB = - mock(IterableHelper.SuccessHandler.class); - IterableAuthRequestCoordinator.Request requestA = - coordinator.enqueue(identityA, null, false, true).getRequestToStart(); - - IterableAuthRequestCoordinator.EnqueueResult enqueueB = - coordinator.enqueue(identityB, callbackB, false, true); - IterableAuthRequestCoordinator.Completion completion = - coordinator.complete(requestA, identityB); - - assertEquals( - IterableAuthRequestCoordinator.EnqueueStatus.QUEUED_FOR_NEW_IDENTITY, - enqueueB.getStatus() - ); - assertFalse(completion.isResultAccepted()); - assertSame(identityB, completion.getNextRequest().getIdentity()); - assertSame(callbackB, completion.getNextRequest().getSuccessCallback()); - assertFalse(completion.getNextRequest().hasFailedPriorAuth()); - } - - @Test - public void latestQueuedIdentityWins() { - Object identityC = new Object(); - IterableAuthRequestCoordinator.Request requestA = - coordinator.enqueue(identityA, null, false, true).getRequestToStart(); - coordinator.enqueue(identityB, null, false, true); - coordinator.enqueue(identityC, null, false, true); - - IterableAuthRequestCoordinator.Completion completion = - coordinator.complete(requestA, identityC); - - assertFalse(completion.isResultAccepted()); - assertSame(identityC, completion.getNextRequest().getIdentity()); - } - - @Test - public void clearingQueuedWorkLeavesNoSuccessor() { - IterableAuthRequestCoordinator.Request requestA = - coordinator.enqueue(identityA, null, false, true).getRequestToStart(); - coordinator.enqueue(identityB, null, false, true); - - coordinator.clearQueued(); - IterableAuthRequestCoordinator.Completion completion = - coordinator.complete(requestA, identityB); - - assertFalse(completion.isResultAccepted()); - assertNull(completion.getNextRequest()); - } - - @Test - public void orphanedCompletionCannotClearTheReplacement() { - IterableAuthRequestCoordinator.Request requestA = - coordinator.enqueue(identityA, null, false, true).getRequestToStart(); - coordinator.enqueue(identityB, null, false, true); - IterableAuthRequestCoordinator.Request requestB = - coordinator.complete(requestA, identityB).getNextRequest(); - - IterableAuthRequestCoordinator.Completion orphanedCompletion = - coordinator.complete(requestA, identityB); - - assertFalse(orphanedCompletion.isResultAccepted()); - assertNull(orphanedCompletion.getNextRequest()); - assertTrue(coordinator.isCurrent(requestB, identityB)); - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java deleted file mode 100644 index 82c4961f4..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthTokenLifecycleTest.java +++ /dev/null @@ -1,491 +0,0 @@ -package com.iterable.iterableapi; - -import com.iterable.iterableapi.unit.PathBasedQueueDispatcher; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.robolectric.annotation.LooperMode; - -import java.io.IOException; -import java.util.Collections; -import java.util.List; -import java.util.Timer; -import java.util.TimerTask; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; - -import okhttp3.mockwebserver.MockWebServer; - -import static android.os.Looper.getMainLooper; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.clearInvocations; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.robolectric.Shadows.shadowOf; -import static org.robolectric.annotation.LooperMode.Mode.PAUSED; - -/** - * Covers how the JWT auth token is created, replaced and recovered as the refresh timer, - * foreground/background transitions and login/logout drive {@link IterableAuthManager}. - */ -@LooperMode(PAUSED) -public class IterableAuthTokenLifecycleTest extends BaseTest { - - /** exp = 2062. */ - private static final String VALID_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI5MTYyMzkwMjJ9.mYtgSqdUIxK8_RnYBTUP4cmpKw83aKi7cMiixF3qMB4"; - /** exp = 2030. */ - private static final String NEW_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE5MTYyMzkwMjJ9.dMD3MLuHTiO-Qy9PvOoMchNM4CzFIgI7jKVrRtlqlM0"; - /** exp = 2018, already expired. */ - private static final String EXPIRED_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE1MTYyNDkwMjJ9.6Yc3QcBGwCdV1sdKmgOtw4D69P_HUoVqEW3YMuEgH8c"; - - private static final String EMAIL = "user@example.com"; - private static final String OTHER_EMAIL = "other@example.com"; - - private MockWebServer server; - private IterableAuthHandler authHandler; - private IterableAuthManager authManager; - private ManualExecutor executor; - - @Before - public void setUp() { - server = new MockWebServer(); - server.setDispatcher(new PathBasedQueueDispatcher()); - IterableApi.overrideURLEndpointPath(server.url("").toString()); - - IterableApi.sharedInstance = new IterableApi(); - authHandler = mock(IterableAuthHandler.class); - doReturn(VALID_JWT).when(authHandler).onAuthTokenRequested(); - - IterableApi.initialize(getContext(), "apiKey", new IterableConfig.Builder() - .setAutoPushRegistration(false) - .setAuthHandler(authHandler) - .build()); - - authManager = IterableApi.getInstance().getAuthManager(); - executor = new ManualExecutor(); - authManager.executor = executor; - } - - @After - public void tearDown() throws IOException { - executor.shutdownNow(); - server.shutdown(); - server = null; - } - - // region token creation - - @Test - public void loginRequestsTokenFromHandlerAndStoresIt() { - login(); - - verify(authHandler).onAuthTokenRequested(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void loginWithSuppliedTokenDoesNotAskTheHandler() { - IterableApi.getInstance().setEmail(EMAIL, VALID_JWT); - settle(); - - verify(authHandler, never()).onAuthTokenRequested(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void handlerReturningNullLeavesNoTokenStored() { - doReturn(null).when(authHandler).onAuthTokenRequested(); - - login(); - - assertNull(IterableApi.getInstance().getAuthToken()); - verify(authHandler).onAuthFailure(failureWithReason(AuthFailureReason.AUTH_TOKEN_NULL)); - } - - @Test - public void handlerThrowingLeavesNoTokenStored() { - doThrow(new RuntimeException("backend down")).when(authHandler).onAuthTokenRequested(); - - login(); - - assertNull(IterableApi.getInstance().getAuthToken()); - verify(authHandler).onAuthFailure(failureWithReason(AuthFailureReason.AUTH_TOKEN_GENERATION_ERROR)); - } - - // endregion - - // region token replacement - - @Test - public void switchingUserReplacesTheStoredToken() { - login(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - - doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); - loginAs(OTHER_EMAIL); - - assertEquals(NEW_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void reLoggingInAsTheSameUserKeepsTheStoredToken() { - login(); - clearInvocations(authHandler); - - doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); - login(); - - verify(authHandler, never()).onAuthTokenRequested(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void failedRefreshKeepsThePreviousToken() { - login(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - - doThrow(new RuntimeException("backend down")).when(authHandler).onAuthTokenRequested(); - authManager.requestNewAuthToken(false, null); - settle(); - - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void logoutClearsTheToken() { - login(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - - IterableApi.getInstance().setEmail(null); - settle(); - - assertNull(IterableApi.getInstance().getAuthToken()); - assertNull(IterableApi.getInstance().getEmail()); - } - - // endregion - - // region recovery after failure - - @Test - public void refreshAfterAFailureRecoversTheToken() { - doThrow(new RuntimeException("backend down")).when(authHandler).onAuthTokenRequested(); - login(); - assertNull(IterableApi.getInstance().getAuthToken()); - - doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); - authManager.requestNewAuthToken(false, null); - settle(); - - assertEquals(NEW_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void expiredTokenIsStoredAndRefreshIsRescheduled() { - doReturn(EXPIRED_JWT).when(authHandler).onAuthTokenRequested(); - - login(); - - assertEquals(EXPIRED_JWT, IterableApi.getInstance().getAuthToken()); - assertTrue("an expired token must leave a refresh armed", isRefreshScheduled()); - } - - @Test - public void malformedTokenReportsPayloadInvalidAndKeepsRefreshing() { - doReturn("not.a.jwt").when(authHandler).onAuthTokenRequested(); - - login(); - - verify(authHandler).onAuthFailure(failureWithReason(AuthFailureReason.AUTH_TOKEN_PAYLOAD_INVALID)); - assertTrue(isRefreshScheduled()); - } - - // endregion - - // region foreground / background - - @Test - public void foregroundWithAValidTokenDoesNotRequestANewOne() { - login(); - clearInvocations(authHandler); - - authManager.onSwitchToBackground(); - authManager.onSwitchToForeground(); - settle(); - - verify(authHandler, never()).onAuthTokenRequested(); - assertEquals(VALID_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void backgroundCancelsTheScheduledRefresh() { - login(); - assertTrue(isRefreshScheduled()); - - authManager.onSwitchToBackground(); - - assertNull("backgrounding must cancel the refresh timer", authManager.timer); - assertNull( - "backgrounding must release refresh ownership", - authManager.scheduledRefreshTask - ); - } - - @Test - public void repeatedForegroundingDoesNotAmplifyTokenRequests() { - login(); - clearInvocations(authHandler); - - for (int i = 0; i < 5; i++) { - authManager.onSwitchToForeground(); - } - settle(); - - verify(authHandler, never()).onAuthTokenRequested(); - assertTrue("foregrounding must leave exactly one refresh armed", isRefreshScheduled()); - } - - @Test - public void tokenRequestIsSkippedWhileBackgrounded() { - login(); - clearInvocations(authHandler); - - authManager.onSwitchToBackground(); - authManager.requestNewAuthToken(false, null); - settle(); - - verify(authHandler, never()).onAuthTokenRequested(); - } - - // endregion - - // region concurrent actors - - @Test - public void concurrentSchedulingArmsOnlyOneRefresh() throws Exception { - CountingTimer timer = installCountingTimer(); - - runConcurrently( - 8, - () -> authManager.scheduleAuthTokenRefresh( - 60_000, - IterableAuthRefreshReason.TOKEN_EXPIRING, - null - ) - ); - - assertEquals(1, timer.liveTaskCount()); - } - - @Test - public void aFiringRefreshRequestsOneTokenAndRearmsOnce() { - login(); - clearInvocations(authHandler); - - CountingTimer timer = armObservableRefresh(); - timer.fireAll(); - settle(); - - verify(authHandler, times(1)).onAuthTokenRequested(); - assertTrue("the refreshed token must leave a new refresh armed", isRefreshScheduled()); - } - - @Test - public void concurrentTokenRequestsCallTheHandlerOnce() throws Exception { - login(); - clearInvocations(authHandler); - - runConcurrently(8, () -> authManager.requestNewAuthToken(false, null)); - settle(); - - verify(authHandler, times(1)).onAuthTokenRequested(); - } - - @Test - public void loginDuringAnInFlightRequestReusesThatRequest() { - authManager.requestNewAuthToken(false, null); - - doReturn(NEW_JWT).when(authHandler).onAuthTokenRequested(); - login(); - - verify(authHandler, times(1)).onAuthTokenRequested(); - assertEquals(NEW_JWT, IterableApi.getInstance().getAuthToken()); - } - - @Test - public void tokenArrivingAfterLogoutIsNotStored() { - login(); - - authManager.requestNewAuthToken(false, null); - IterableApi.getInstance().setEmail(null); - settle(); - - assertNull("a token resolved after logout must not restore the session", - IterableApi.getInstance().getAuthToken()); - } - - // endregion - - private void login() { - loginAs(EMAIL); - } - - private void loginAs(String email) { - IterableApi.getInstance().setEmail(email); - settle(); - } - - /** Drains the auth executor and the main looper until both are idle. */ - private void settle() { - for (int i = 0; i < 10; i++) { - boolean ranTask = executor.runAll() > 0; - shadowOf(getMainLooper()).runToEndOfTasks(); - if (!ranTask && !executor.hasPendingTasks()) { - return; - } - } - } - - private boolean isRefreshScheduled() { - return authManager.scheduledRefreshTask != null; - } - - private CountingTimer installCountingTimer() { - CountingTimer timer = new CountingTimer(); - authManager.timer = timer; - return timer; - } - - /** Discards whatever refresh is already armed and arms one the test can fire on demand. */ - private CountingTimer armObservableRefresh() { - authManager.clearRefreshTimer(); - CountingTimer timer = installCountingTimer(); - authManager.scheduleAuthTokenRefresh( - 60_000, - IterableAuthRefreshReason.TOKEN_EXPIRING, - null - ); - assertEquals(1, timer.liveTaskCount()); - return timer; - } - - private void runConcurrently(int threadCount, Runnable action) throws Exception { - CountDownLatch start = new CountDownLatch(1); - CountDownLatch done = new CountDownLatch(threadCount); - for (int i = 0; i < threadCount; i++) { - new Thread(() -> { - try { - start.await(); - action.run(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } finally { - done.countDown(); - } - }).start(); - } - start.countDown(); - assertTrue(done.await(10, TimeUnit.SECONDS)); - } - - private static AuthFailure failureWithReason(AuthFailureReason reason) { - return org.mockito.ArgumentMatchers.argThat(failure -> failure != null && failure.failureReason == reason); - } - - /** Records scheduled tasks instead of running them, so a test can count and fire them. */ - private static class CountingTimer extends Timer { - private final List tasks = new CopyOnWriteArrayList<>(); - - CountingTimer() { - super(true); - super.cancel(); - } - - @Override - public void schedule(TimerTask task, long delay) { - tasks.add(task); - } - - @Override - public void cancel() { - tasks.clear(); - } - - int liveTaskCount() { - return tasks.size(); - } - - void fireAll() { - for (TimerTask task : tasks) { - task.run(); - } - } - } - - /** - * Executor that queues submitted work until the test drains it. Unlike Robolectric's - * InlineExecutorService this never runs the task inside submit(), which would deadlock: - * requestNewAuthToken submits while holding the auth manager's monitor, and the task - * re-enters that monitor via queueExpirationRefresh. - */ - private static class ManualExecutor extends java.util.concurrent.AbstractExecutorService { - private final java.util.Queue pending = new java.util.concurrent.ConcurrentLinkedQueue<>(); - private volatile boolean shutdown; - - @Override - public void execute(Runnable command) { - if (!shutdown) { - pending.add(command); - } - } - - int runAll() { - int count = 0; - Runnable task; - while ((task = pending.poll()) != null) { - task.run(); - count++; - } - return count; - } - - boolean hasPendingTasks() { - return !pending.isEmpty(); - } - - @Override - public void shutdown() { - shutdown = true; - } - - @Override - public List shutdownNow() { - shutdown = true; - pending.clear(); - return Collections.emptyList(); - } - - @Override - public boolean isShutdown() { - return shutdown; - } - - @Override - public boolean isTerminated() { - return shutdown && pending.isEmpty(); - } - - @Override - public boolean awaitTermination(long timeout, TimeUnit unit) { - return isTerminated(); - } - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt index 41039aace..ae35197cc 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt @@ -178,81 +178,6 @@ 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 testReadAuthTokenDistinguishesAStoredValue() { - `when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())) - .thenReturn("encrypted_stored-token") - - val result = keychain.readAuthToken() - - assertEquals(KeychainReadResult.Value("stored-token"), result) - } - - @Test - fun testReadAuthTokenDistinguishesATimeoutFromAMissingValue() { - `when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())) - .thenReturn("slow_token") - `when`(mockEncryptor.decrypt(eq("slow_token"))).thenAnswer { - Thread.sleep(700) - "stored-token" - } - - val result = keychain.readAuthToken() - - assertEquals(KeychainReadResult.TimedOut, result) - } - - @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 From 7b19e5e9b09b286df73bafb9d2ca52762c7e0151 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Fri, 21 Aug 2026 10:55:04 +0100 Subject: [PATCH 11/11] [SDK-547] Prevent stale refresh dispatch --- .../iterableapi/IterableAuthManager.java | 29 ++++--- .../IterableAuthRefreshOwnershipTest.java | 77 +++++++++++++++++++ 2 files changed, 97 insertions(+), 9 deletions(-) diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java index cfd053bc9..9244c1d7e 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java @@ -358,18 +358,14 @@ synchronized void scheduleAuthTokenRefresh( final TimerTask refreshTask = new TimerTask() { @Override public void run() { - if (!claimRefreshTask(this, reason)) { + if (!isCurrentRefreshTask(this, reason)) { return; } - IterableLogger.d(TAG, "auth_refresh action=fire reason=" + reason); if (api.getEmail() != null || api.getUserId() != null) { - api.getAuthManager().requestNewAuthToken( - false, - successCallback, - reason.ignoresRetryPolicy() - ); + dispatchRefreshTask(this, reason, successCallback); } else { + releaseRefreshTask(this); IterableLogger.w( TAG, "auth_refresh action=skip reason=" @@ -407,7 +403,7 @@ public void run() { } } - private synchronized boolean claimRefreshTask( + private synchronized boolean isCurrentRefreshTask( TimerTask task, IterableAuthRefreshReason reason ) { @@ -420,10 +416,25 @@ private synchronized boolean claimRefreshTask( ); return false; } + return true; + } + private synchronized void dispatchRefreshTask( + TimerTask task, + IterableAuthRefreshReason reason, + IterableHelper.SuccessHandler successCallback + ) { + if (!isCurrentRefreshTask(task, reason)) { + return; + } scheduledRefreshTask = null; scheduledRefreshReason = null; - return true; + IterableLogger.d(TAG, "auth_refresh action=fire reason=" + reason); + api.getAuthManager().requestNewAuthToken( + false, + successCallback, + reason.ignoresRetryPolicy() + ); } private synchronized void releaseRefreshTask(TimerTask task) { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java index 6cc5e6320..b795e4f29 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAuthRefreshOwnershipTest.java @@ -9,10 +9,12 @@ import java.util.List; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertEquals; @@ -113,6 +115,48 @@ public void staleTaskCannotRunOrClearItsReplacement() { assertEquals(1, authManager.requestCount.get()); } + @Test + public void replacingTokenWhileRefreshChecksIdentityCancelsOldRefresh() throws Exception { + CountDownLatch identityCheckStarted = new CountDownLatch(1); + CountDownLatch allowOldTokenRefreshToContinue = new CountDownLatch(1); + pauseFirstIdentityCheckUntil( + identityCheckStarted, + allowOldTokenRefreshToContinue + ); + + // The old token's expiration refresh starts checking for an identified user. + TimerTask oldTokenRefresh = scheduleRefresh( + 1000, + IterableAuthRefreshReason.TOKEN_EXPIRING + ); + ExecutorService taskRunner = Executors.newSingleThreadExecutor(); + Future oldTokenRefreshRun = taskRunner.submit(oldTokenRefresh::run); + try { + assertTrue(identityCheckStarted.await(5, TimeUnit.SECONDS)); + + // The app receives a new token and schedules its expiration refresh. + TimerTask newTokenRefresh = replaceRefreshForNewToken(2000); + + // The old token's refresh resumes after the new token has replaced it. + allowOldTokenRefreshToContinue.countDown(); + oldTokenRefreshRun.get(5, TimeUnit.SECONDS); + + // The old refresh must not request another token or disturb the new refresh. + assertEquals(0, authManager.requestCount.get()); + assertSame(newTokenRefresh, authManager.scheduledRefreshTask); + + // The new token's refresh can still dispatch normally. + newTokenRefresh.run(); + + assertEquals(1, authManager.requestCount.get()); + assertNull(authManager.scheduledRefreshTask); + assertNull(authManager.scheduledRefreshReason); + } finally { + allowOldTokenRefreshToContinue.countDown(); + taskRunner.shutdownNow(); + } + } + @Test public void duplicateScheduleKeepsOriginalCallbackAndPolicy() { RetainingTimer timer = new RetainingTimer(); @@ -190,6 +234,39 @@ public void scheduledLifecycleRefreshStillIgnoresPausedRetries() { ); } + private void pauseFirstIdentityCheckUntil( + CountDownLatch identityCheckStarted, + CountDownLatch resumeIdentityCheck + ) { + AtomicInteger identityCheckCount = new AtomicInteger(); + when(api.getEmail()).thenAnswer(invocation -> { + if (identityCheckCount.getAndIncrement() == 0) { + identityCheckStarted.countDown(); + assertTrue(resumeIdentityCheck.await(5, TimeUnit.SECONDS)); + } + return "user@example.com"; + }); + } + + private TimerTask scheduleRefresh( + long delay, + IterableAuthRefreshReason reason + ) { + RetainingTimer timer = new RetainingTimer(); + authManager.timer = timer; + authManager.scheduleAuthTokenRefresh(delay, reason, null); + return timer.lastTask(); + } + + private TimerTask replaceRefreshForNewToken(long delay) { + // Mirrors queueExpirationRefresh() after a new token is stored. + authManager.clearRefreshTimer(); + return scheduleRefresh( + delay, + IterableAuthRefreshReason.TOKEN_EXPIRING + ); + } + private static class RecordingAuthManager extends IterableAuthManager { private final AtomicInteger requestCount = new AtomicInteger(); private IterableHelper.SuccessHandler lastSuccessCallback;