From 2138763457d2cc504ae97ec702ad83a571e8a755 Mon Sep 17 00:00:00 2001 From: Nan Date: Mon, 8 Jun 2026 18:38:09 -0700 Subject: [PATCH] fix: prevent stale FetchUser from clobbering the current user's identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an on-new-session Fetch User for a previous user (e.g. a cached anonymous user) is still pending and login() switches the current user, the in-flight Fetch User completing cleared and re-hydrated local data unconditionally — wiping the external_id login had just set on the new current user. The subsequent user-2 (409) conflict recovery then built a Create User with external_id: nil, minting a stray anonymous user and silently dropping the login (and any addEmail/tags applied afterward). In executeFetchUserRequest, early-return when the fetched user is no longer the current user: such a response is stale, so clearing/hydrating local data would only clobber the now-current user. Add regression tests for both the non-current (must not touch current user) and current (must still clear + hydrate) cases. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Source/Executors/OSUserExecutor.swift | 7 ++- .../Executors/UserExecutorTests.swift | 57 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 6c2f3f0e2..4fb6094a4 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -448,6 +448,12 @@ extension OSUserExecutor { OneSignalCoreImpl.sharedClient().execute(request) { response in self.removeFromQueue(request) + // A fetch for a user that is no longer current is stale + guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) else { + self.executePendingRequests() + return + } + if let response = response { // Clear local data in preparation for hydration OneSignalUserManagerImpl.sharedInstance.clearUserData() @@ -455,7 +461,6 @@ extension OSUserExecutor { // If this is a on-new-session's fetch user call, check that the subscription still exists if request.onNewSession, - OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel), let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId, let subscriptionObjects = self.parseSubscriptionObjectResponse(response) { var subscriptionExists = false diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift index f914bb4be..f1eb5bbb3 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift @@ -187,4 +187,61 @@ final class UserExecutorTests: XCTestCase { XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self)) XCTAssertTrue(mocks.newRecordsState.records.isEmpty) } + + /** + Regression test for a login race that landed identity (and subsequent user updates) data on the wrong user. + + When an on-new-session Fetch User request for a *previous* user (e.g. a cached anonymous user) is still + pending and a `login()` switches the current user, the in-flight Fetch User must NOT clear the new current + user's data. + */ + func testFetchUser_forNonCurrentUser_doesNotClearCurrentUserData() { + /* Setup */ + let mocks = Mocks() + + // The current user has just logged in with an external_id (userB). + let currentUser = OneSignalUserMocks.setUserManagerInternalUser(externalId: userB_EUID, onesignalId: userB_OSID) + + // A stale on-new-session Fetch User is in flight for a different, no-longer-current user (userA), + // and its response only carries an onesignal_id (as an anonymous user's would). + let staleIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()) + mocks.client.setMockResponseForRequest( + request: "", + response: MockUserRequests.testIdentityPayload(onesignalId: userA_OSID, externalId: nil) + ) + + /* When */ + mocks.userExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: userA_OSID, identityModel: staleIdentityModel, onNewSession: true) + OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5) + + /* Then */ + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestFetchUser.self)) + // The current user's external_id must be intact — the stale fetch must not have cleared it. + XCTAssertEqual(currentUser.identityModel.externalId, userB_EUID) + XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance._user?.identityModel.externalId, userB_EUID) + } + + /** + The normal new-session Fetch User for the *current* user must still clear stale local data before hydrating + from the response, so the `isCurrentUser` guard added for the race above does not regress the common path. + */ + func testFetchUser_forCurrentUser_stillClearsStaleData() { + /* Setup */ + let mocks = Mocks() + let currentUser = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID) + // A stale local alias that is not present in the server response and should be cleared by the fetch. + currentUser.identityModel.addAliases(["stale_label": "stale_value"]) + + MockUserRequests.setDefaultFetchUserResponseForHydration(with: mocks.client, externalId: userA_EUID) + + /* When */ + mocks.userExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: userA_OSID, identityModel: currentUser.identityModel, onNewSession: false) + OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5) + + /* Then */ + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestFetchUser.self)) + // clearUserData() ran for the current user: the stale alias is gone and server aliases are hydrated. + XCTAssertNil(currentUser.identityModel.aliases["stale_label"]) + XCTAssertEqual(currentUser.identityModel.externalId, userA_EUID) + } }