Skip to content

Commit 7c7b360

Browse files
nan-liclaude
andcommitted
fix: prevent stale FetchUser from clobbering the current user's identity (#1672)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 80ccd53)
1 parent f45bde6 commit 7c7b360

2 files changed

Lines changed: 65 additions & 1 deletion

File tree

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,12 @@ extension OSUserExecutor {
602602
OneSignalCoreImpl.sharedClient().execute(request) { response in
603603
self.removeFromRequestQueueAndPersist(request)
604604

605+
// A fetch for a user that is no longer current is stale
606+
guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) else {
607+
self.executePendingRequests()
608+
return
609+
}
610+
605611
if let response = response {
606612
// Clear local data in preparation for hydration
607613
// TODO: JWT 🔐 the following line feels wrong... maybe the user's changed by now
@@ -610,7 +616,6 @@ extension OSUserExecutor {
610616

611617
// If this is a on-new-session's fetch user call, check that the subscription still exists
612618
if request.onNewSession,
613-
OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel),
614619
let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId,
615620
let subscriptionObjects = self.parseSubscriptionObjectResponse(response) {
616621
var subscriptionExists = false

iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,4 +433,63 @@ final class UserExecutorTests: XCTestCase {
433433
// >= because start() may fire incidental requests (e.g. language update from _user?.update())
434434
XCTAssertGreaterThanOrEqual(mocks.client.networkRequestCount, 3)
435435
}
436+
437+
/**
438+
Regression test for a login race that landed identity (and subsequent user updates) data on the wrong user.
439+
440+
When an on-new-session Fetch User request for a *previous* user (e.g. a cached anonymous user) is still
441+
pending and a `login()` switches the current user, the in-flight Fetch User must NOT clear the new current
442+
user's data.
443+
*/
444+
func testFetchUser_forNonCurrentUser_doesNotClearCurrentUserData() {
445+
/* Setup */
446+
let mocks = Mocks()
447+
mocks.setAuthRequired(false)
448+
449+
// The current user has just logged in with an external_id (userB).
450+
let currentUser = OneSignalUserMocks.setUserManagerInternalUser(externalId: userB_EUID, onesignalId: userB_OSID)
451+
452+
// A stale on-new-session Fetch User is in flight for a different, no-longer-current user (userA),
453+
// and its response only carries an onesignal_id (as an anonymous user's would).
454+
let staleIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer())
455+
mocks.client.setMockResponseForRequest(
456+
request: "<OSRequestFetchUser with onesignal_id: \(userA_OSID)>",
457+
response: MockUserRequests.testIdentityPayload(onesignalId: userA_OSID, externalId: nil)
458+
)
459+
460+
/* When */
461+
mocks.userExecutor.fetchUser(onesignalId: userA_OSID, identityModel: staleIdentityModel, onNewSession: true)
462+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
463+
464+
/* Then */
465+
XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestFetchUser.self))
466+
// The current user's external_id must be intact — the stale fetch must not have cleared it.
467+
XCTAssertEqual(currentUser.identityModel.externalId, userB_EUID)
468+
XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance._user?.identityModel.externalId, userB_EUID)
469+
}
470+
471+
/**
472+
The normal new-session Fetch User for the *current* user must still clear stale local data before hydrating
473+
from the response, so the `isCurrentUser` guard added for the race above does not regress the common path.
474+
*/
475+
func testFetchUser_forCurrentUser_stillClearsStaleData() {
476+
/* Setup */
477+
let mocks = Mocks()
478+
mocks.setAuthRequired(false)
479+
let currentUser = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
480+
// A stale local alias that is not present in the server response and should be cleared by the fetch.
481+
currentUser.identityModel.addAliases(["stale_label": "stale_value"])
482+
483+
MockUserRequests.setDefaultFetchUserResponseForHydration(with: mocks.client, externalId: userA_EUID)
484+
485+
/* When */
486+
mocks.userExecutor.fetchUser(onesignalId: userA_OSID, identityModel: currentUser.identityModel, onNewSession: false)
487+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
488+
489+
/* Then */
490+
XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestFetchUser.self))
491+
// clearUserData() ran for the current user: the stale alias is gone and server aliases are hydrated.
492+
XCTAssertNil(currentUser.identityModel.aliases["stale_label"])
493+
XCTAssertEqual(currentUser.identityModel.externalId, userA_EUID)
494+
}
436495
}

0 commit comments

Comments
 (0)