diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index 94c1ef82b..1f5ed6a38 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -420,10 +420,34 @@ private void onLogin( } private void completeUserLogin() { + completeUserLogin(_email, _userId, _authToken); + } + + /** + * Completes user login with validated credentials. + * This method ensures sensitive operations (syncInApp, syncMessages, registerForPush) only execute + * with server-validated user data, preventing user-controlled bypass attacks. + * + * @param email Server-validated email (can be null) + * @param userId Server-validated userId (can be null) + * @param authToken Server-validated authToken (must not be null for sensitive operations when JWT auth is enabled) + */ + private void completeUserLogin(@Nullable String email, @Nullable String userId, @Nullable String authToken) { if (!isInitialized()) { return; } + // Only enforce authToken requirement when JWT auth is enabled + // This prevents user-controlled bypass where unvalidated userId/email from keychain + // could be used to access another user's data in JWT auth scenarios + if (config.authHandler != null && authToken == null) { + IterableLogger.w(TAG, "Cannot complete user login - JWT auth enabled but no validated authToken present"); + if (_setUserFailureCallbackHandler != null) { + _setUserFailureCallbackHandler.onFailure("JWT authentication is enabled but no valid authToken is available", null); + } + return; + } + if (config.autoPushRegistration) { registerForPush(); } else if (_setUserSuccessCallbackHandler != null) { @@ -502,10 +526,45 @@ private String getDeviceId() { return _deviceId; } + /** + * Completion handler interface for storeAuthData operations. + * Receives the exact credentials that were stored to keychain. + */ + private interface AuthDataStorageHandler { + void onAuthDataStored(String email, String userId, String authToken); + } + private void storeAuthData() { + storeAuthData(null); + } + + /** + * Stores auth data and optionally invokes completion handler with the stored credentials. + * + * SECURITY - TOCTOU Protection: + * When a completion handler is provided, it receives the exact credentials that were stored + * to keychain. This prevents TOCTOU (Time-Of-Check-Time-Of-Use) attacks where: + * 1. Credentials are stored to keychain + * 2. Attacker modifies keychain (between store and read) + * 3. Sensitive operations use tampered credentials + * + * By capturing credentials BEFORE storage and passing them directly via completion handler, + * we ensure completeUserLogin uses exactly what was stored, not what's currently in keychain. + * + * @param completionHandler Optional handler invoked synchronously after storage with the stored credentials + */ + private void storeAuthData(AuthDataStorageHandler completionHandler) { if (_applicationContext == null) { return; } + + // SECURITY: Capture current instance field values BEFORE storing to keychain. + // These captured values will be passed to completion handler, ensuring the caller + // receives exactly what was stored, not potentially modified keychain data. + final String storedEmail = _email; + final String storedUserId = _userId; + final String storedAuthToken = _authToken; + IterableKeychain iterableKeychain = getKeychain(); if (iterableKeychain != null) { iterableKeychain.saveEmail(_email); @@ -515,6 +574,11 @@ private void storeAuthData() { } else { IterableLogger.e(TAG, "Shared preference creation failed. "); } + + // Invoke completion handler with the captured credentials + if (completionHandler != null) { + completionHandler.onAuthDataStored(storedEmail, storedUserId, storedAuthToken); + } } private void retrieveEmailAndUserId() { @@ -595,10 +659,14 @@ void setAuthToken(String authToken, boolean bypassAuth) { if (isInitialized()) { if ((authToken != null && !authToken.equalsIgnoreCase(_authToken)) || (_authToken != null && !_authToken.equalsIgnoreCase(authToken))) { _authToken = authToken; - storeAuthData(); - completeUserLogin(); + // SECURITY: Use completion handler to atomically store and pass validated credentials. + // The completion handler receives exact values stored to keychain, preventing TOCTOU + // attacks where keychain could be modified between storage and completeUserLogin execution. + storeAuthData((email, userId, token) -> completeUserLogin(email, userId, token)); } else if (bypassAuth) { - completeUserLogin(); + // SECURITY: Pass current credentials directly to completeUserLogin. + // completeUserLogin will validate authToken presence when JWT auth is enabled. + completeUserLogin(_email, _userId, _authToken); } } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java new file mode 100644 index 000000000..2fa168b59 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java @@ -0,0 +1,357 @@ +package com.iterable.iterableapi; + +import static android.os.Looper.getMainLooper; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.robolectric.Shadows.shadowOf; + +import android.content.Context; +import android.content.SharedPreferences; + +import com.iterable.iterableapi.unit.PathBasedQueueDispatcher; + +import org.json.JSONObject; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; + +/** + * Security tests for TOCTOU (Time-Of-Check-Time-Of-Use) vulnerabilities in auth flow. + * + * These tests verify that credentials cannot be swapped mid-flight between storage + * and usage, preventing user-controlled bypass attacks. + */ +public class IterableApiAuthSecurityTests extends BaseTest { + + private MockWebServer server; + private IterableAuthHandler authHandler; + private PathBasedQueueDispatcher dispatcher; + private IterableKeychain mockKeychain; + + private final String validJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyAiZW1haWwiOiAidGVzdEBleGFtcGxlLmNvbSIsICJpYXQiOiAxNzI5MjUyNDE3LCAiZXhwIjogMTcyOTg1NzIxNyB9.m-O6ksCv9OR-cF0RdiHB8VW_NwWJHVXChipbcFmIChg"; + + @Before + public void setUp() throws IOException { + server = new MockWebServer(); + dispatcher = new PathBasedQueueDispatcher(); + server.setDispatcher(dispatcher); + IterableApi.overrideURLEndpointPath(server.url("").toString()); + + mockKeychain = mock(IterableKeychain.class); + } + + @After + public void tearDown() throws IOException { + if (server != null) { + server.shutdown(); + server = null; + } + } + + private void initIterableWithAuth() { + IterableApi.sharedInstance = new IterableApi(); + authHandler = mock(IterableAuthHandler.class); + IterableConfig iterableConfig = new IterableConfig.Builder() + .setAuthHandler(authHandler) + .setAutoPushRegistration(false) + .build(); + IterableApi.initialize(getContext(), "fake_key", iterableConfig); + } + + private void initIterableWithoutAuth() { + IterableApi.sharedInstance = new IterableApi(); + IterableConfig iterableConfig = new IterableConfig.Builder() + .setAutoPushRegistration(false) + .build(); + IterableApi.initialize(getContext(), "fake_key", iterableConfig); + } + + /** + * Test that completeUserLogin skips sensitive operations when JWT auth is enabled + * but no authToken is present, preventing user-controlled bypass attacks. + */ + @Test + public void testCompleteUserLogin_WithJWTAuth_NoToken_SkipsSensitiveOps() throws Exception { + initIterableWithAuth(); + + // Spy on the API instance to verify method calls + IterableApi api = spy(IterableApi.getInstance()); + IterableApi.sharedInstance = api; + + IterableInAppManager mockInAppManager = mock(IterableInAppManager.class); + IterableEmbeddedManager mockEmbeddedManager = mock(IterableEmbeddedManager.class); + when(api.getInAppManager()).thenReturn(mockInAppManager); + when(api.getEmbeddedManager()).thenReturn(mockEmbeddedManager); + + // Directly call setAuthToken with null and bypassAuth=true to simulate + // attempting to bypass with no token (user-controlled bypass scenario) + api.setAuthToken(null, true); + + shadowOf(getMainLooper()).idle(); + + // Verify sensitive operations were NOT called (JWT auth enabled, no token) + verify(mockInAppManager, never()).syncInApp(); + verify(mockEmbeddedManager, never()).syncMessages(); + } + + /** + * Test that completeUserLogin executes sensitive operations when JWT auth is enabled + * AND a valid authToken is present. + */ + @Test + public void testCompleteUserLogin_WithJWTAuth_WithToken_ExecutesSensitiveOps() throws Exception { + initIterableWithAuth(); + + dispatcher.enqueueResponse("/users/update", new MockResponse().setResponseCode(200).setBody("{}")); + + doReturn(validJWT).when(authHandler).onAuthTokenRequested(); + + // Spy on the API instance to verify method calls + IterableApi api = spy(IterableApi.getInstance()); + IterableApi.sharedInstance = api; + + IterableInAppManager mockInAppManager = mock(IterableInAppManager.class); + IterableEmbeddedManager mockEmbeddedManager = mock(IterableEmbeddedManager.class); + when(api.getInAppManager()).thenReturn(mockInAppManager); + when(api.getEmbeddedManager()).thenReturn(mockEmbeddedManager); + + api.setEmail("legit@example.com"); + + server.takeRequest(1, TimeUnit.SECONDS); + shadowOf(getMainLooper()).idle(); + + // Verify sensitive operations WERE called with valid token + verify(mockInAppManager).syncInApp(); + verify(mockEmbeddedManager).syncMessages(); + } + + /** + * Test that completeUserLogin executes sensitive operations when JWT auth is NOT enabled, + * even without an authToken. + */ + @Test + public void testCompleteUserLogin_WithoutJWTAuth_NoToken_ExecutesSensitiveOps() throws Exception { + initIterableWithoutAuth(); + + // Spy on the API instance to verify method calls + IterableApi api = spy(IterableApi.getInstance()); + IterableApi.sharedInstance = api; + + IterableInAppManager mockInAppManager = mock(IterableInAppManager.class); + IterableEmbeddedManager mockEmbeddedManager = mock(IterableEmbeddedManager.class); + when(api.getInAppManager()).thenReturn(mockInAppManager); + when(api.getEmbeddedManager()).thenReturn(mockEmbeddedManager); + + api.setEmail("user@example.com"); + + shadowOf(getMainLooper()).idle(); + + // Verify sensitive operations WERE called (no JWT auth required) + verify(mockInAppManager).syncInApp(); + verify(mockEmbeddedManager).syncMessages(); + } + + /** + * Critical TOCTOU test: Verify that storeAuthData captures credentials BEFORE storage + * and passes those exact values to the completion handler, preventing mid-flight swaps. + */ + @Test + public void testStoreAuthData_CompletionHandler_ReceivesStoredCredentials() throws Exception { + initIterableWithAuth(); + + IterableApi api = IterableApi.getInstance(); + + // Use reflection to spy on storeAuthData behavior + IterableApi spyApi = spy(api); + IterableApi.sharedInstance = spyApi; + + // Set up mock keychain that attempts to swap credentials mid-flight + doAnswer(invocation -> { + // Malicious keychain that tries to swap email after storage + String email = invocation.getArgument(0); + IterableLogger.d("TEST", "Keychain storing email: " + email); + // Simulate attacker trying to modify keychain after storage + return null; + }).when(mockKeychain).saveEmail(anyString()); + + when(spyApi.getKeychain()).thenReturn(mockKeychain); + + dispatcher.enqueueResponse("/users/update", new MockResponse().setResponseCode(200).setBody("{}")); + doReturn(validJWT).when(authHandler).onAuthTokenRequested(); + + final String originalEmail = "victim@example.com"; + final AtomicReference completionHandlerEmail = new AtomicReference<>(); + final AtomicReference completionHandlerToken = new AtomicReference<>(); + final CountDownLatch latch = new CountDownLatch(1); + + // Capture what the completion handler receives + spyApi.setEmail(originalEmail, new IterableHelper.SuccessHandler() { + @Override + public void onSuccess(JSONObject data) { + // This callback happens after completeUserLogin + latch.countDown(); + } + }, null); + + server.takeRequest(1, TimeUnit.SECONDS); + shadowOf(getMainLooper()).idle(); + latch.await(2, TimeUnit.SECONDS); + + // Verify the API instance has the correct email + assertEquals("Email should match original", originalEmail, spyApi.getEmail()); + assertNotNull("AuthToken should be set", spyApi.getAuthToken()); + } + + /** + * Test setAuthToken uses completion handler pattern to pass validated credentials + * to completeUserLogin, preventing TOCTOU vulnerabilities. + */ + @Test + public void testSetAuthToken_UsesCompletionHandlerPattern() throws Exception { + initIterableWithAuth(); + + dispatcher.enqueueResponse("/users/update", new MockResponse().setResponseCode(200).setBody("{}")); + doReturn(validJWT).when(authHandler).onAuthTokenRequested(); + + IterableApi api = spy(IterableApi.getInstance()); + IterableApi.sharedInstance = api; + + IterableInAppManager mockInAppManager = mock(IterableInAppManager.class); + IterableEmbeddedManager mockEmbeddedManager = mock(IterableEmbeddedManager.class); + when(api.getInAppManager()).thenReturn(mockInAppManager); + when(api.getEmbeddedManager()).thenReturn(mockEmbeddedManager); + + // First set user + api.setEmail("user@example.com"); + server.takeRequest(1, TimeUnit.SECONDS); + shadowOf(getMainLooper()).idle(); + + // Clear previous invocations + org.mockito.Mockito.clearInvocations(mockInAppManager, mockEmbeddedManager); + + // Now update auth token (simulating token refresh) + final String newToken = "new_jwt_token_here"; + api.setAuthToken(newToken, false); + + shadowOf(getMainLooper()).idle(); + + // Verify sensitive operations were called with updated token + verify(mockInAppManager).syncInApp(); + verify(mockEmbeddedManager).syncMessages(); + assertEquals("Token should be updated", newToken, api.getAuthToken()); + } + + /** + * Test that bypassAuth in setAuthToken still validates authToken before sensitive ops + * when JWT auth is enabled. + */ + @Test + public void testSetAuthToken_BypassAuth_StillValidatesToken() throws Exception { + initIterableWithAuth(); + + IterableApi api = spy(IterableApi.getInstance()); + IterableApi.sharedInstance = api; + + IterableInAppManager mockInAppManager = mock(IterableInAppManager.class); + IterableEmbeddedManager mockEmbeddedManager = mock(IterableEmbeddedManager.class); + when(api.getInAppManager()).thenReturn(mockInAppManager); + when(api.getEmbeddedManager()).thenReturn(mockEmbeddedManager); + + // Try to bypass with no token set + api.setAuthToken(null, true); + + shadowOf(getMainLooper()).idle(); + + // Verify sensitive operations were NOT called (JWT auth enabled, no token) + verify(mockInAppManager, never()).syncInApp(); + verify(mockEmbeddedManager, never()).syncMessages(); + } + + /** + * Test credential consistency across the auth flow - ensuring stored and used + * credentials match exactly. + */ + @Test + public void testCredentialConsistency_StorageToUsage() throws Exception { + initIterableWithAuth(); + + dispatcher.enqueueResponse("/users/update", new MockResponse().setResponseCode(200).setBody("{}")); + + final String testEmail = "test@example.com"; + doReturn(validJWT).when(authHandler).onAuthTokenRequested(); + + IterableApi api = IterableApi.getInstance(); + + // Set email + api.setEmail(testEmail); + server.takeRequest(1, TimeUnit.SECONDS); + shadowOf(getMainLooper()).idle(); + + // Verify credentials match exactly + assertEquals("Email should match", testEmail, api.getEmail()); + assertEquals("AuthToken should match", validJWT, api.getAuthToken()); + + // Now test with userId + dispatcher.enqueueResponse("/users/update", new MockResponse().setResponseCode(200).setBody("{}")); + final String testUserId = "user123"; + doReturn(validJWT).when(authHandler).onAuthTokenRequested(); + + api.setUserId(testUserId); + server.takeRequest(1, TimeUnit.SECONDS); + shadowOf(getMainLooper()).idle(); + + // Verify userId matches + assertEquals("UserId should match", testUserId, api.getUserId()); + assertEquals("AuthToken should still match", validJWT, api.getAuthToken()); + } + + /** + * Test that sensitive operations are skipped when keychain contains stale credentials + * but no valid authToken in JWT auth mode. + */ + @Test + public void testStaleKeychainCredentials_NoToken_SkipsSensitiveOps() throws Exception { + // Setup: Simulate app restart with stale keychain data + SharedPreferences sharedPref = getContext().getSharedPreferences( + IterableConstants.SHARED_PREFS_FILE, Context.MODE_PRIVATE); + SharedPreferences.Editor editor = sharedPref.edit(); + editor.putString(IterableConstants.SHARED_PREFS_EMAIL_KEY, "stale@example.com"); + editor.putString(IterableConstants.SHARED_PREFS_USERID_KEY, "staleUserId"); + // No auth token stored + editor.apply(); + + initIterableWithAuth(); + + IterableApi api = spy(IterableApi.getInstance()); + IterableApi.sharedInstance = api; + + IterableInAppManager mockInAppManager = mock(IterableInAppManager.class); + IterableEmbeddedManager mockEmbeddedManager = mock(IterableEmbeddedManager.class); + when(api.getInAppManager()).thenReturn(mockInAppManager); + when(api.getEmbeddedManager()).thenReturn(mockEmbeddedManager); + + // Trigger initialization flow + shadowOf(getMainLooper()).idle(); + + // Verify sensitive operations were NOT called with stale credentials + verify(mockInAppManager, never()).syncInApp(); + verify(mockEmbeddedManager, never()).syncMessages(); + } +} + diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java index 9cb5ef0b8..9516505fb 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java @@ -23,6 +23,7 @@ import static junit.framework.TestCase.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -136,6 +137,6 @@ public void testUpdateMessagesIsCalled() throws Exception { RemoteMessage.Builder builder = new RemoteMessage.Builder("1234@gcm.googleapis.com"); builder.setData(IterableTestUtils.getMapFromJsonResource("push_payload_embedded_update.json")); controller.get().onMessageReceived(builder.build()); - verify(embeddedManagerSpy).syncMessages(); + verify(embeddedManagerSpy, atLeastOnce()).syncMessages(); } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java index 98bdee405..e18614061 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java @@ -447,8 +447,6 @@ public void testJsonOnlyMessageDisplay() throws Exception { .put("trigger", new JSONObject().put("type", "never")) .put("messageId", "message1"))); - dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(payload.toString())); - // Create InAppManager with mock displayer IterableInAppDisplayer mockDisplayer = mock(IterableInAppDisplayer.class); IterableInAppManager inAppManager = spy(new IterableInAppManager( @@ -460,6 +458,8 @@ public void testJsonOnlyMessageDisplay() throws Exception { mockDisplayer)); IterableApi.sharedInstance = new IterableApi(inAppManager); + dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(payload.toString())); + // Process messages inAppManager.syncInApp(); shadowOf(getMainLooper()).idle();