From ebcadb28bb141647f7cb255e8ee1c66659119fc9 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 11 Sep 2026 12:35:00 -0700 Subject: [PATCH 01/13] fix: [SDK-5137] keep a restored Identify User that waits on its own Create User `uncacheUserRequests` on 5.7-main kept a restored Identify User only when Identity Verification was on or the Request could prepare at once, and the Identify prepare needs the `onesignal_id` of the user it identifies. That dropped the archive an offline first launch with a `login` leaves behind, the anonymous Create User followed by the Identify User that logs A in, on relaunch whenever the requirement was off or unknown, so the login was lost. On main the Identify was kept whenever its identify model was already in the repo, which a restored Create User ahead of it guarantees, and it went out once that response supplied the id. PR6 (#1711) collapsed those cases into one check. Restore main's rule. Keep the Identify when its identify model is in the repo, or while the requirement is not known to be off so reshape can decide, otherwise only when it can be sent as is. Keep main's guard too. Once the requirement is known off, `reshapeInvalidRequests` drops an Identify whose user never received an `onesignal_id` and has nothing queued to supply one, because the FIFO loop stops at the first unexecutable Request and would otherwise retry it for the life of the install. Three tests in UserExecutorTests: the archive above with the requirement off, the same archive turning out to require auth, and a never-preparable Identify kept while unknown then dropped once auth is known to be off. The first two fail without the fix. --- .../Source/Executors/OSUserExecutor.swift | 54 ++++++++- .../Executors/UserExecutorTests.swift | 110 ++++++++++++++++++ 2 files changed, 161 insertions(+), 3 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 66c6450bb..15d51c8fc 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -67,9 +67,13 @@ class OSUserExecutor { Runs on every send because `refreshIfUnknown` can raise `requirement` with no event; reads the live model because this executor sends nothing while `requirement` is unknown. + + With the requirement off there is nothing to reshape, but a login kept at start while the requirement + was still unknown may never become sendable; see `dropIdentifyUsersThatCanNeverPrepare`. */ private func reshapeInvalidRequests() { guard identityVerificationService.ivBehaviorActive else { + dropIdentifyUsersThatCanNeverPrepare() return } @@ -120,6 +124,44 @@ class OSUserExecutor { return request is OSRequestFetchIdentityBySubscription } + /** + With the requirement off, an Identify User whose user never received an `onesignal_id`, and has no + queued Create User or Fetch Identity By Subscription left to supply one, can never prepare. Drop it + rather than let it hold the queue and block the logins behind it. Only reachable for a login kept at + start while the requirement was still unknown; `uncacheUserRequests` drops the same Request when the + requirement is already known to be off. + */ + private func dropIdentifyUsersThatCanNeverPrepare() { + guard identityVerificationService.requirement == .off else { + return + } + let modelIdsAwaitingAnId = Set(userRequestQueue.compactMap { request -> String? in + if let createUser = request as? OSRequestCreateUser { + return createUser.identityModel.modelId + } + if let fetchIdentity = request as? OSRequestFetchIdentityBySubscription { + return fetchIdentity.identityModel.modelId + } + return nil + }) + let kept = userRequestQueue.filter { request in + guard let identifyUser = request as? OSRequestIdentifyUser, + identifyUser.identityModelToIdentify.onesignalId == nil, + !modelIdsAwaitingAnId.contains(identifyUser.identityModelToIdentify.modelId) + else { + return true + } + let reason = "its user never received an onesignal_id and nothing queued can supply one" + OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor dropped \(identifyUser), \(reason)") + return false + } + guard kept.count != userRequestQueue.count else { + return + } + userRequestQueue = kept + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: userRequestQueue) + } + /// Read in requests from the cache, do not read in FetchUser requests as this is not needed. private func uncacheUserRequests() { var userRequestQueue: [OSUserRequest] = [] @@ -159,9 +201,15 @@ class OSUserExecutor { req.identityModelToUpdate = updateInRepo } - // `prepareForExecution` is false under IV so `reshapeInvalidRequests` can promote - // this login; do not treat that as a permanent drop. - if auth.ivBehaviorActive || request.prepareForExecution(newRecordsState: newRecordsState, auth: auth) { + // Keep the login when the user it identifies is known to the repo: a restored Create User + // ahead of it put the model there, and that response supplies the `onesignal_id` prepare + // needs. Keep it too while the requirement is not known to be off, since + // `reshapeInvalidRequests` decides what a login made under Identity Verification becomes + // once the requirement is known. Otherwise only a Request that can be sent as is stays: + // one that can never prepare would hold the queue and block the logins behind it. + if identifyInRepo != nil + || identityVerificationService.requirement != .off + || request.prepareForExecution(newRecordsState: newRecordsState, auth: auth) { if identifyInRepo == nil { addIdentityModel(req.identityModelToIdentify) } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift index 3f85a3774..21e146a70 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift @@ -439,6 +439,84 @@ final class UserExecutorTests: XCTestCase { XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) } + /// The archive an offline first launch with a `login` leaves behind: the anonymous Create User has not + /// been sent, so its user has no `onesignal_id` yet. The Identify User behind it has to wait for that + /// response rather than be dropped at start. + func testRestoredIdentifyUserBehindItsUnsentCreateUserIsSentOnceTheCreateUserCompletes() { + /* Setup */ + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: false) + let user = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: nil) + let createUser = makeUnsentAnonymousCreateUserRequest() + cacheUserRequests([createUser, makeIdentifyUserRequest(identifying: createUser.identityModel, updating: user.identityModel)]) + + /* When */ + let mocks = Mocks { + MockUserRequests.setDefaultCreateAnonUserResponses(with: $0) + MockUserRequests.setDefaultIdentifyUserResponses(with: $0, externalId: userA_EUID, conflicted: false) + } + OneSignalCoreMocks.waitUntil("Restored Identify User was not sent after its Create User") { + mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self) + } + + /* Then */ + XCTAssertTrue(mocks.client.executedRequests.first is OSRequestCreateUser, "the Create User has to go out first") + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self, expectedCount: 1)) + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self, expectedCount: 1)) + } + + /// Same archive with the requirement unknown at start. Once auth turns out to be required, reshape + /// drops the anonymous Create User and promotes the Identify User into the Create User `login` would + /// have made. + func testRestoredIdentifyUserBehindItsUnsentCreateUserBecomesACreateUserWhenAuthIsRequired() { + /* Setup */ + makeRequirementUnknown() + let user = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: nil) + user.identityModel.jwtBearerToken = "token-a" + let createUser = makeUnsentAnonymousCreateUserRequest() + cacheUserRequests([createUser, makeIdentifyUserRequest(identifying: createUser.identityModel, updating: user.identityModel)]) + let mocks = Mocks { MockUserRequests.setDefaultCreateUserResponses(with: $0, externalId: userA_EUID) } + allowAsyncWorkToRun() + XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self), + "nothing may be sent while the requirement is unknown") + + /* When */ + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) + OneSignalCoreMocks.waitUntil("Restored Identify User was not reshaped into a Create User") { + mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self) + } + + /* Then */ + let createUsers = mocks.client.executedRequests.compactMap { $0 as? OSRequestCreateUser } + XCTAssertEqual(createUsers.map { $0.identityModel.externalId }, [userA_EUID], "only the promoted Create User for A may go out") + XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) + } + + /// A login kept at start while the requirement was unknown, whose user never received an + /// `onesignal_id` and has no Create User left to supply one, can never prepare once auth is known to + /// be off. It has to be dropped rather than hold the queue, or every login behind it is stranded. + func testRestoredIdentifyUserThatCanNeverPrepareIsDroppedOnceAuthIsKnownToBeOff() { + /* Setup */ + makeRequirementUnknown() + let user = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: nil) + let neverCreated = OSIdentityModel(aliases: nil, changeNotifier: OSEventProducer()) + cacheUserRequests([makeIdentifyUserRequest(identifying: neverCreated, updating: user.identityModel)]) + let mocks = Mocks { MockUserRequests.setDefaultCreateUserResponses(with: $0, externalId: userB_EUID) } + + /* When */ + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: false) + // A login behind the dead Identify User has to go out. + let userB = mocks.createUserInstance(externalId: userB_EUID) + OneSignalUserManagerImpl.sharedInstance._user = userB + mocks.userExecutor.createUser(userB) + OneSignalCoreMocks.waitUntil("Create User behind the dead Identify User was not sent") { + mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self) + } + + /* Then */ + XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self, expectedCount: 1)) + } + /// `login` promotes while the requirement is still unknown, so turning out to require auth must not /// strand that login: it becomes the Create User it would have been. func testInSessionIdentifyUserBecomesACreateUserWhenIdentityVerificationIsRequired() { @@ -505,6 +583,38 @@ final class UserExecutorTests: XCTestCase { originalPushToken: nil ) } + + private func makeIdentifyUserRequest( + identifying identityModelToIdentify: OSIdentityModel, + updating identityModelToUpdate: OSIdentityModel + ) -> OSRequestIdentifyUser { + return OSRequestIdentifyUser( + aliasLabel: OS_EXTERNAL_ID, + aliasId: userA_EUID, + identityModelToIdentify: identityModelToIdentify, + identityModelToUpdate: identityModelToUpdate + ) + } + + /// A Create User for an anonymous user that has not been sent, so its user has no `onesignal_id`. + private func makeUnsentAnonymousCreateUserRequest() -> OSRequestCreateUser { + let pushModel = OSSubscriptionModel( + type: .push, address: nil, subscriptionId: nil, reachable: false, isDisabled: false, changeNotifier: OSEventProducer() + ) + return OSRequestCreateUser( + identityModel: OSIdentityModel(aliases: nil, changeNotifier: OSEventProducer()), + propertiesModel: OSPropertiesModel(changeNotifier: OSEventProducer()), + pushSubscriptionModel: pushModel, + originalPushToken: nil + ) + } + + /// `OneSignalUserMocks.reset()` hydrates the requirement off for non-IV tests, so a test that starts + /// unknown has to clear both the shared config and its cache. + private func makeRequirementUnknown() { + OneSignalUserDefaults.initShared().removeValue(forKey: OSUD_USE_IDENTITY_VERIFICATION) + OSCoreMocks.resetSharedJwtConfig() + } } /// Upgrade decode of `addsNewRecords` on a cached Create User. From 1d1f05039be343ba79f95fa72e12c84ab6349b9f Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 11 Sep 2026 13:45:00 -0700 Subject: [PATCH 02/13] fix: [SDK-5137] report only the current user when a Request hydrates its model `OSIdentityModel.hydrateModel` fired the user-state observer for whichever model it hydrated. Stepping over a parked Create User (#1711) lets a later login proceed, so that parked Request can now complete after another user is current, and its response reported the earlier user as signed in and persisted that pair, so nothing later re-reported the current user. Move the fire out of the model to the three executor sites that hydrate an identity model (Create User and Fetch User through `parseFetchUserResponse`, Identify User, and Fetch Identity By Subscription), through `OSUserStateSnapshot.fireUserStateChangedIfCurrent`, which reports only when the model still belongs to the current user. The hydration itself stays, since Requests queued behind that user need the `onesignal_id`. Test in a new UserStateReportingTests, since UserJwtLifecycleTests is at SwiftLint's type body limit: login(A) with no token, login(B) with one, then answer A's ask; the app hears nothing new and the persisted pair still names B. Fails without the fix. --- .../OneSignal.xcodeproj/project.pbxproj | 4 + .../Source/Executors/OSUserExecutor.swift | 4 + .../Source/OSIdentityModel.swift | 25 +++- .../UserStateReportingTests.swift | 130 ++++++++++++++++++ 4 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj index f1a5ce93e..be5c18e74 100644 --- a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj +++ b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj @@ -357,6 +357,7 @@ 7AFE856C2368DDB80091D6A5 /* OSFocusCallParams.m in Sources */ = {isa = PBXBuildFile; fileRef = 7AFE856A2368DDB80091D6A5 /* OSFocusCallParams.m */; }; 7AFE856D2368DDB80091D6A5 /* OSFocusCallParams.m in Sources */ = {isa = PBXBuildFile; fileRef = 7AFE856A2368DDB80091D6A5 /* OSFocusCallParams.m */; }; 7EB69F3B404D0AEF46EC1536 /* UserJwtLifecycleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BFE2F960129386AFA6D5F41 /* UserJwtLifecycleTests.swift */; }; + E42087CB1AB15481E55D34FE /* UserStateReportingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FDA6355B75EDC9A3100B98F5 /* UserStateReportingTests.swift */; }; 8D2F4893453206700BB60F85 /* OSOperationRepoTestSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5221EEDBA5A74BD565490D52 /* OSOperationRepoTestSupport.swift */; }; 8E949FF4C7A7A2C7182E53EA /* OSUserJwtConfigTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E9376A4957E9090C748BCB18 /* OSUserJwtConfigTests.swift */; }; 911E2CBD1E398AB3003112A4 /* UnitTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 911E2CBC1E398AB3003112A4 /* UnitTests.m */; }; @@ -1591,6 +1592,7 @@ 5BC1DE632C90BB9000CA8807 /* OSIamFetchReadyCondition.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIamFetchReadyCondition.swift; sourceTree = ""; }; 5BC1DE672C90C23E00CA8807 /* OSConsistencyManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSConsistencyManagerTests.swift; sourceTree = ""; }; 5BFE2F960129386AFA6D5F41 /* UserJwtLifecycleTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = UserJwtLifecycleTests.swift; sourceTree = ""; }; + FDA6355B75EDC9A3100B98F5 /* UserStateReportingTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = UserStateReportingTests.swift; sourceTree = ""; }; 6552F2A6DF7776B0582CFAEF /* OSUserJwtConfig.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = OSUserJwtConfig.swift; sourceTree = ""; }; 67ECA2928D863073B785F93F /* IamFetchIdentityVerificationTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = IamFetchIdentityVerificationTests.swift; sourceTree = ""; }; 6A8BBA843AFC81A4940CF7CC /* OSUserJwtRepo.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = OSUserJwtRepo.swift; sourceTree = ""; }; @@ -2523,6 +2525,7 @@ 047D8F5E1095A20C9C54FD33 /* OSRequestAuthTests.swift */, 3016921C1F6B7B7793F67567 /* RequestPathEncodingTests.swift */, 5BFE2F960129386AFA6D5F41 /* UserJwtLifecycleTests.swift */, + FDA6355B75EDC9A3100B98F5 /* UserStateReportingTests.swift */, ); path = OneSignalUserTests; sourceTree = ""; @@ -4708,6 +4711,7 @@ AAFA2D46E6C5FD3D14D39F27 /* OSRequestAuthTests.swift in Sources */, 23D66BEB40CE76DFF89744A3 /* RequestPathEncodingTests.swift in Sources */, 7EB69F3B404D0AEF46EC1536 /* UserJwtLifecycleTests.swift in Sources */, + E42087CB1AB15481E55D34FE /* UserStateReportingTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 15d51c8fc..fd4e6ff2c 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -474,6 +474,7 @@ extension OSUserExecutor { if let identityObject = self.parseIdentityObjectResponse(response), let onesignalId = identityObject[OS_ONESIGNAL_ID] { request.identityModel.hydrate(identityObject) + OSUserStateSnapshot.fireUserStateChangedIfCurrent(request.identityModel) // Fetch this user's data if it is the current user guard OneSignalUserManagerImpl.sharedInstance.currentUser(matching: request.identityModel.modelId) != nil @@ -536,6 +537,7 @@ extension OSUserExecutor { request.aliasLabel: request.aliasId ] request.identityModelToUpdate.hydrate(aliases) + OSUserStateSnapshot.fireUserStateChangedIfCurrent(request.identityModelToUpdate) // the anonymous user has been identified, still need to Fetch User as we cleared local data if OneSignalUserManagerImpl.sharedInstance.currentUser(matching: request.identityModelToUpdate.modelId) != nil { @@ -665,8 +667,10 @@ extension OSUserExecutor { // If this was a create user, it hydrates the onesignal_id of the request's identityModel // The model in the store may be different, and it may be waiting on the onesignal_id of this previous model + // Only a current user is reported to the app; a parked Create User can complete after a switch. if let identityObject = parseIdentityObjectResponse(response) { identityModel.hydrate(identityObject) + OSUserStateSnapshot.fireUserStateChangedIfCurrent(identityModel) if addNewRecords, let onesignalId = identityObject[OS_ONESIGNAL_ID] { newRecordsState.add(onesignalId) } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift index 6df132faf..42a461afb 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift @@ -161,21 +161,34 @@ class OSIdentityModel: OSModel { } OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSIdentityModel hydrateModel with aliases: \(remoteAliases)") - let newOnesignalId = remoteAliases[OS_ONESIGNAL_ID] - let newExternalId = remoteAliases[OS_EXTERNAL_ID] - + // Reporting the user to the app is the executor's call, since only a current user may be reported. internalAddAliases(remoteAliases) - OSUserStateSnapshot.fireUserStateChanged(newOnesignalId: newOnesignalId, newExternalId: newExternalId) } } /** Owns the last user state the app was told about, so the observer only hears real changes. - Hydration is the usual source, but `logout` under Identity Verification also reports here: it creates - no user on the server, so there is no hydration to carry the news that nobody is signed in. + The User executor reports a hydrated current user through `fireUserStateChangedIfCurrent`, and `logout` + under Identity Verification also reports here: it creates no user on the server, so there is no + hydration to carry the news that nobody is signed in. */ enum OSUserStateSnapshot { + /** + Reports the hydrated user to the app, but only while that user is still current. A Create User or + Identify User for a user the app has since switched away from still hydrates its model, since the + Requests queued behind it need the `onesignal_id`, but the app must not hear that user as signed in, + and the persisted pair must keep naming the current user, or the current user's real state would + later read as unchanged and go unreported. + */ + static func fireUserStateChangedIfCurrent(_ identityModel: OSIdentityModel) { + guard OneSignalUserManagerImpl.sharedInstance.currentUser(matching: identityModel.modelId) != nil else { + OneSignalLog.onesignalLog(.LL_DEBUG, message: "OSUserStateSnapshot not reporting a hydrated user who is no longer current") + return + } + fireUserStateChanged(newOnesignalId: identityModel.onesignalId, newExternalId: identityModel.externalId) + } + /// Fires the user observer if `onesignal_id` OR `external_id` differs from the last reported pair. static func fireUserStateChanged(newOnesignalId: String?, newExternalId: String?) { let prevOnesignalId = OneSignalUserDefaults.initShared().getSavedString(forKey: OS_SNAPSHOT_ONESIGNAL_ID, defaultValue: nil) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift new file mode 100644 index 000000000..0d8347990 --- /dev/null +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift @@ -0,0 +1,130 @@ +/* + Modified MIT License + + Copyright 2026 OneSignal + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + 1. The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + 2. All copies of substantial portions of the Software may only be used in connection + with services provided by OneSignal. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + */ + +import XCTest +import OneSignalCore +import OneSignalCoreMocks +import OneSignalOSCoreMocks +import OneSignalUserMocks +@testable import OneSignalOSCore +@testable import OneSignalUser + +private class MockUserStateObserver: NSObject, OSUserStateObserver { + var states: [OSUserState] = [] + + func onUserStateDidChange(state: OSUserChangedState) { + states.append(state.current) + } +} + +/** + What the app's `OSUserStateObserver` hears, and what the persisted snapshot names, once a Request can + complete for a user who is no longer current. + */ +final class UserStateReportingTests: XCTestCase { + private var client = MockOneSignalClient() + private var observer = MockUserStateObserver() + + override func setUpWithError() throws { + OneSignalCoreMocks.clearUserDefaults() + OneSignalUserMocks.reset() + OneSignalIdentifiers.currentAppId = "test-app-id" + + client = MockOneSignalClient() + MockUserRequests.setDefaultCreateUserResponses(with: client, externalId: userA_EUID) + MockUserRequests.setDefaultCreateUserResponses(with: client, externalId: userB_EUID) + OneSignalCoreImpl.setSharedClient(client) + + // Held strongly for the test's lifetime: OSObservable keeps observers weakly. + observer = MockUserStateObserver() + OneSignalUserManagerImpl.sharedInstance.addObserver(observer) + } + + override func tearDownWithError() throws { + // A Request still in flight would land mid-next-test and hydrate the shared models under it. + OneSignalCoreMocks.waitUntil("A Request was still in flight at teardown") { self.clientIsIdle } + OneSignalUserManagerImpl.sharedInstance.removeObserver(observer) + OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = false + OneSignalCoreMocks.clearUserDefaults() + } + + /** + Stepping over a parked Create User lets a later login proceed, so the parked one can complete after + another user is current. Its model still hydrates, since Requests queued behind it need the + `onesignal_id`, but the app has to keep hearing the current user, and the persisted pair has to keep + naming the current user, or the app is told the wrong user is signed in. + */ + func testAParkedCreateUserThatCompletesAfterAUserSwitchDoesNotReportThatUser() { + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) + + // Parks for want of a token, which asks the app. + OneSignalUserManagerImpl.sharedInstance.login(externalId: userA_EUID, token: nil) + allowAsyncWorkToRun() + // Steps over the parked Create User and becomes the current, reported user. + OneSignalUserManagerImpl.sharedInstance.login(externalId: userB_EUID, token: "token-b") + waitForTheLoginToSettle() + XCTAssertEqual(observer.states.last?.externalId, userB_EUID) + let reportsBeforeA = observer.states.count + + // Answers the ask, so A's Create User goes out while B is current. + OneSignalUserManagerImpl.sharedInstance.updateUserJwt(externalId: userA_EUID, token: "token-a") + OneSignalCoreMocks.waitUntil("A's parked Create User was not sent") { + self.client.executedRequests.contains { ($0 as? OSRequestCreateUser)?.identityModel.externalId == userA_EUID } + } + OneSignalCoreMocks.waitUntil("A's Create User was still in flight") { self.clientIsIdle } + allowAsyncWorkToRun(seconds: 0.1) + + // Hydrated, so anything queued for A has its onesignal_id. + XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance.identityModelRepo.get(externalId: userA_EUID)?.onesignalId, userA_OSID) + // Not reported: the app hears nothing new, and the persisted pair still names B. + XCTAssertEqual(observer.states.count, reportsBeforeA, "the app must not hear about A: \(observer.states)") + XCTAssertEqual(observer.states.last?.externalId, userB_EUID) + XCTAssertEqual(OneSignalUserDefaults.initShared().getSavedString(forKey: OS_SNAPSHOT_EXTERNAL_ID, defaultValue: nil), userB_EUID) + XCTAssertEqual(OneSignalUserDefaults.initShared().getSavedString(forKey: OS_SNAPSHOT_ONESIGNAL_ID, defaultValue: nil), userB_OSID) + } + + // MARK: - Waits + + private var clientIsIdle: Bool { + return client.completedRequests.count == client.startedRequests.count + } + + /** + Outlasts every effect of an identified login, including the Fetch User its Create User starts. A + fetch landing later would re-report the current user and hide a wrong report made in between. + */ + private func waitForTheLoginToSettle() { + OneSignalCoreMocks.waitUntil("The login did not reach a reported user") { + OneSignalUserManagerImpl.sharedInstance.user.identityModel.onesignalId != nil + && self.observer.states.contains { $0.onesignalId != nil } + && self.client.hasCompletedRequestOfType(OSRequestFetchUser.self) + && self.clientIsIdle + } + allowAsyncWorkToRun(seconds: 0.1) + OneSignalCoreMocks.waitUntil("The login left a Request in flight") { self.clientIsIdle } + } +} From 331366ce54ac8b48c4b39a84f4e463026bcc4abe Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 11 Sep 2026 13:49:46 -0700 Subject: [PATCH 03/13] fix: [SDK-5137] retry a Create User the cool-down held instead of waiting for a token `_executePendingRequests` steps over a Request the auth layer parked for a token, so a login for another user behind it is not stranded, and it schedules no retry for a wait only the app can end. It told a park apart from any other failed prepare with `awaitsToken`, which only asked whether the owner had a token. A Create User still inside the new-records cool-down on its push subscription fails to prepare before the auth layer runs, so with Identity Verification on and no token yet it was stepped over as if parked: nobody was asked, nothing retried, and the login sat in the queue until the next launch. Replace `awaitsToken` with `parkedForToken`, which answers whether the last authorization of that Request actually parked it. `OSRequestAuth` keeps the parked Requests in a weak table: `park` adds one, every authorization starts by forgetting it so the entry reflects the latest attempt, and the executor consumes the answer right after a failed prepare. A prepare that fails before authorizing therefore reads as not parked and gets the delayed retry. Tests: three unit tests on the mark's lifecycle in OSRequestAuthTests, and UserExecutorRetryTests, a new file since UserExecutorTests is at SwiftLint's type body limit, holding a Create User in the cool-down and asserting the app is asked once it lifts. Fails without the fix. --- .../OneSignal.xcodeproj/project.pbxproj | 4 + .../Source/Executors/OSUserExecutor.swift | 4 +- .../OneSignalUser/Source/OSRequestAuth.swift | 37 ++++++-- .../Executors/UserExecutorRetryTests.swift | 91 +++++++++++++++++++ .../OSRequestAuthTests.swift | 35 +++++++ 5 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorRetryTests.swift diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj index be5c18e74..53dad875b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj +++ b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj @@ -247,6 +247,7 @@ 3CEE93572B7C78FD008440BD /* OneSignalCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DE7D17E627026B95002D3A5D /* OneSignalCore.framework */; }; 3CEE93582B7C78FE008440BD /* OneSignalCore.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DE7D17E627026B95002D3A5D /* OneSignalCore.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; 3CF11E3D2C6D6155002856F5 /* UserExecutorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */; }; + 354E0C59BA9B18437C36215B /* UserExecutorRetryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6C310B6C569E96C204F8CD68 /* UserExecutorRetryTests.swift */; }; 3CF11E402C6E6DE2002856F5 /* MockNewRecordsState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF11E3F2C6E6DE2002856F5 /* MockNewRecordsState.swift */; }; 3CF1A5632C669EA40056B3AA /* OSNewRecordsState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF1A5622C669EA40056B3AA /* OSNewRecordsState.swift */; }; 3CF8629E28A183F900776CA4 /* OSIdentityModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF8629D28A183F900776CA4 /* OSIdentityModel.swift */; }; @@ -1516,6 +1517,7 @@ 3CEE90A62BFE6ABD00B0FB5B /* OSPropertiesSupportedProperty.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSPropertiesSupportedProperty.swift; sourceTree = ""; }; 3CEE90A82C000BD500B0FB5B /* OneSignalRequest+UnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OneSignalRequest+UnitTests.swift"; sourceTree = ""; }; 3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserExecutorTests.swift; sourceTree = ""; }; + 6C310B6C569E96C204F8CD68 /* UserExecutorRetryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserExecutorRetryTests.swift; sourceTree = ""; }; 3CF11E3F2C6E6DE2002856F5 /* MockNewRecordsState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockNewRecordsState.swift; sourceTree = ""; }; 3CF1A5622C669EA40056B3AA /* OSNewRecordsState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSNewRecordsState.swift; sourceTree = ""; }; 3CF8629D28A183F900776CA4 /* OSIdentityModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIdentityModel.swift; sourceTree = ""; }; @@ -2542,6 +2544,7 @@ isa = PBXGroup; children = ( 3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */, + 6C310B6C569E96C204F8CD68 /* UserExecutorRetryTests.swift */, 3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */, 3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */, FE740F9E6B87D215510B5982 /* ExecutorAnonymousPurgeTests.swift */, @@ -4694,6 +4697,7 @@ files = ( 3CB331682F281679000E1801 /* CustomEventsIntegrationTests.swift in Sources */, 3CF11E3D2C6D6155002856F5 /* UserExecutorTests.swift in Sources */, + 354E0C59BA9B18437C36215B /* UserExecutorRetryTests.swift in Sources */, 3C67F77A2BEB2B710085A0F0 /* SwitchUserIntegrationTests.swift in Sources */, 3CC063EE2B6D7FE8002BB07F /* OneSignalUserTests.swift in Sources */, 3CA93BC7300B0100000724B3 /* SubscriptionModelConcurrencyTests.swift in Sources */, diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index fd4e6ff2c..0c7c05e1d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -300,7 +300,9 @@ class OSUserExecutor { else { // Only the app can end this wait (`updateUserJwt` → `storeJwt`); do not poll for it. // A login for another user behind this one must not be stranded, so step over it. - if self.auth.awaitsToken(request) { + // Anything else that stops a prepare, the cool-down or an id still to arrive, resolves + // on its own, and the delayed retry below is what picks it up. + if self.auth.parkedForToken(request) { awaitingToken = true continue } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift index 5678d9747..835c0258e 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift @@ -57,13 +57,15 @@ protocol OSRequestAuthorizing: AnyObject { func authorize(_ request: OSUserRequest) -> Bool /** - Returns `true` if the Request's owner has no token to sign with, which is why the two methods above - parked it. Reads only: it does not ask the app for a token, so call it after one of them has. + Whether the last authorization of this Request parked it, because its owner has no token to sign + with. Consumes the answer, so ask once per `prepareForExecution`, right after it returns false. Lets a caller that stops at its first unsendable Request tell "nothing can send until the app hands - over a token for this user" from "not addressable yet", which resolves on its own. + over a token for this user" from "not addressable yet", which resolves on its own: an app id that has + not arrived, a user still inside the new-records cool-down, or an `onesignal_id` a queued Create User + has yet to supply. Those fail before the two methods above run, so only a park may skip the retry. */ - func awaitsToken(_ request: OSUserRequest) -> Bool + func parkedForToken(_ request: OSUserRequest) -> Bool /** Parks the token an unauthorized response rejected and clears `sentToClient` so the Request is @@ -119,6 +121,15 @@ final class OSRequestAuth: OSRequestAuthorizing { private let identityVerificationService: OSIdentityVerificationService private let jwt: OSUserJwtProviding + /** + Requests whose last authorization `park` held. Weak, so a Request that leaves its queue leaves this + too. Every authorization starts by forgetting the Request, so the entry reflects the latest attempt, + and `parkedForToken` removes it, so a prepare that fails before authorizing reads as not parked. + Shared by every executor's queue, hence the lock. + */ + private let parkedRequests = NSHashTable.weakObjects() + private let parkedRequestsLock = NSLock() + var ivBehaviorActive: Bool { return identityVerificationService.ivBehaviorActive } @@ -129,6 +140,7 @@ final class OSRequestAuth: OSRequestAuthorizing { } func authorizeUserScoped(_ request: OSUserRequest, legacyAlias: OSAliasPair) -> OSAliasPair? { + forgetPark(of: request) guard ivBehaviorActive else { return legacyAlias } @@ -148,6 +160,7 @@ final class OSRequestAuth: OSRequestAuthorizing { } func authorize(_ request: OSUserRequest) -> Bool { + forgetPark(of: request) guard ivBehaviorActive else { return true } @@ -167,11 +180,18 @@ final class OSRequestAuth: OSRequestAuthorizing { return true } - func awaitsToken(_ request: OSUserRequest) -> Bool { - guard ivBehaviorActive, let externalId = request.ownerExternalId else { - return false + func parkedForToken(_ request: OSUserRequest) -> Bool { + return parkedRequestsLock.withLock { + guard parkedRequests.contains(request) else { + return false + } + parkedRequests.remove(request) + return true } - return jwt.validJwt(externalId: externalId) == nil + } + + private func forgetPark(of request: OSUserRequest) { + parkedRequestsLock.withLock { parkedRequests.remove(request) } } /** @@ -180,6 +200,7 @@ final class OSRequestAuth: OSRequestAuthorizing { SDK holding none with nothing to reject. The repo keeps this to one ask per external ID per session. */ private func park(_ request: OSUserRequest, ownedBy externalId: String) { + parkedRequestsLock.withLock { parkedRequests.add(request) } // Log only on the ask that reaches the app; later prepareForExecution retries stay quiet. if jwt.askForToken(externalId: externalId) { OneSignalLog.onesignalLog(.LL_DEBUG, message: "OSRequestAuth: holding \(request) until \(externalId) has a token") diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorRetryTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorRetryTests.swift new file mode 100644 index 000000000..a621edb58 --- /dev/null +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorRetryTests.swift @@ -0,0 +1,91 @@ +/* + Modified MIT License + + Copyright 2026 OneSignal + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + 1. The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + 2. All copies of substantial portions of the Software may only be used in connection + with services provided by OneSignal. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + */ + +import XCTest +import OneSignalCore +import OneSignalOSCore +import OneSignalCoreMocks +import OneSignalOSCoreMocks +import OneSignalUserMocks +@testable import OneSignalUser + +/** + What the User executor retries on its own, and what it waits on the app for. A parked Request waits + for a token that only `updateUserJwt` can supply; anything else that stops a prepare resolves on its + own and has to be retried, or a login sits in the queue until the next launch. + */ +final class UserExecutorRetryTests: XCTestCase { + private var client = MockOneSignalClient() + private var newRecordsState = MockNewRecordsState() + + override func setUpWithError() throws { + OneSignalCoreMocks.clearUserDefaults() + OneSignalUserMocks.reset() + OneSignalIdentifiers.currentAppId = "test-app-id" + + client = MockOneSignalClient() + OneSignalCoreImpl.setSharedClient(client) + newRecordsState = MockNewRecordsState() + // Presence is the hold: the production timer is a no-op under TEST. + newRecordsState.holdWhilePresent = true + } + + override func tearDownWithError() throws { + OneSignalCoreMocks.clearUserDefaults() + } + + private func makeExecutor() -> OSUserExecutor { + return OSUserExecutor( + newRecordsState: newRecordsState, + identityVerificationService: OneSignalUserManagerImpl.sharedInstance.identityVerificationService, + auth: OneSignalUserManagerImpl.sharedInstance.requestAuth + ) + } + + /** + A Create User held by the new-records cool-down on its push subscription fails to prepare before + the auth layer can park it, so nobody is asked for a token. It has to be retried once the cool-down + passes, at which point it parks and asks, rather than be stepped over as if it were already parked. + */ + func testACreateUserHeldByTheCoolDownIsRetriedUntilItCanParkAndAsk() { + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) + let jwtRepo = OneSignalUserManagerImpl.sharedInstance.userJwtRepo + let user = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: nil) + newRecordsState.add(testPushSubId) + let executor = makeExecutor() + + executor.createUser(user) + allowAsyncWorkToRun() + XCTAssertFalse(jwtRepo.pendingTokenAsks().contains(userA_EUID), "held by the cool-down, so nobody may be asked yet") + + newRecordsState.holdWhilePresent = false + OneSignalCoreMocks.waitUntil("The app was not asked for a token once the cool-down passed") { + jwtRepo.pendingTokenAsks().contains(userA_EUID) + } + XCTAssertFalse(client.hasExecutedRequestOfType(OSRequestCreateUser.self), "nothing signs a Create User whose owner has no token") + } +} diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift index 62455a1d7..c91107deb 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift @@ -255,6 +255,41 @@ final class OSRequestAuthTests: XCTestCase { XCTAssertNil(request.authorizationHeader) } + // MARK: - parkedForToken + + /// The answer is consumed, so the executor asks once per failed prepare and sees a fresh answer next time. + func testParkedForTokenIsTrueOnceAfterAPark() { + let auth = makeAuth(requiresUserAuth: true) + let request = StubUserRequest(ownerExternalId: "user-a") + + XCTAssertFalse(auth.parkedForToken(request), "nothing has parked it yet") + XCTAssertFalse(auth.authorize(request)) + XCTAssertTrue(auth.parkedForToken(request)) + XCTAssertFalse(auth.parkedForToken(request), "consumed by the read before") + } + + /// A signed authorization is not a park, so a prepare that fails after it failed for some other reason. + func testParkedForTokenIsFalseAfterASignedAuthorization() { + let auth = makeAuth(requiresUserAuth: true) + jwt.tokens["user-a"] = "token-a" + let request = StubUserRequest(ownerExternalId: "user-a") + + XCTAssertTrue(auth.authorize(request)) + XCTAssertFalse(auth.parkedForToken(request)) + } + + /// The entry reflects the latest authorization: a park does not outlive a later signed attempt. + func testASignedAuthorizationClearsAnEarlierPark() { + let auth = makeAuth(requiresUserAuth: true) + let request = StubUserRequest(ownerExternalId: "user-a") + XCTAssertFalse(auth.authorize(request)) + + jwt.tokens["user-a"] = "token-a" + XCTAssertTrue(auth.authorizeUserScoped(request, legacyAlias: OSAliasPair(OS_ONESIGNAL_ID, "osid")) != nil) + + XCTAssertFalse(auth.parkedForToken(request)) + } + // MARK: - handleUnauthorized func testHandleUnauthorizedInvalidatesTheSignedTokenAndRequeuesTheRequest() { From d9d6f1105b56201aa18b3d5fee2f4dc449a5a22f Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 11 Sep 2026 13:52:21 -0700 Subject: [PATCH 04/13] fix: [SDK-5137] keep external_id across the clear before a Fetch User hydrates `OSIdentityModel.clearData` blanked every alias in preparation for the Fetch User response that hydrates them, and `executeFetchUserRequest` runs that clear right before parsing the response, on the response thread. A properties or subscription Delta built in that gap on another queue read the current user's `externalId` as nil, and with Identity Verification on `OSOperationRepo.enqueueDelta` drops an anonymous Delta before it is ever persisted, so the change was lost for good. Keep `external_id` in `clearData`. The fetch that follows is by `onesignal_id`, so it cannot change who the user is, and its response overwrites the alias anyway. One behavior change comes with it: a fetch response that omits `external_id` no longer demotes the local user to anonymous. Only a server-side unlink produces one, and the next `login` corrects it, so that reads as the right trade against silently dropping an identified user's work. Test in OSIdentityModelTests: an identified model keeps `externalId` across `clearData` and loses every other alias. Fails without the fix. The ownership convention comment in OSUserRequest.swift no longer cites the blanked aliases as the reason for the owner stamp. --- .../Source/OSIdentityModel.swift | 9 ++++++++- .../Source/Requests/OSUserRequest.swift | 9 +++++---- .../OSIdentityModelTests.swift | 19 ++++++++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift index 42a461afb..08d35e3ef 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift @@ -134,10 +134,17 @@ class OSIdentityModel: OSModel { /** Called to clear the model's data in preparation for hydration via a fetch user call. + + `external_id` stays. The fetch that follows is by `onesignal_id`, so it cannot change who the user + is, and its response overwrites the alias anyway. Blanking it would let work built in the gap before + that response, on another queue, read this user as anonymous: a Delta stamped with no owner is + dropped under Identity Verification before it is ever persisted. A response that omits `external_id` + no longer reads as anonymous either; only a server-side unlink produces one, and the next `login` + corrects it. */ func clearData() { lock.withLock { - self.aliases = [:] + self.aliases = self.aliases.filter { $0.key == OS_EXTERNAL_ID } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index 7842b4708..b2a26a5ee 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -55,10 +55,11 @@ extension OSUserRequest { owner's `external_id` as of when the Request was built, and both the purge and the token lookup judge it by that rather than by its `identityModel`. - The live model cannot answer the question. `clearUserData` empties an Identity Model's aliases before - a fetch response hydrates them, so for that window an identified user reads as anonymous and a purge - running alongside it would delete signed work. The stamp also matches how `OSDelta` carries - `externalId`, which keeps a Delta and the Request built from it judged the same way. + The live model is not the record of who the work was for. Its aliases are cleared and hydrated again + around every fetch (only `external_id` survives the clear, see `OSIdentityModel.clearData`), and the + owner has to be what it was when the work was built, not what the model reads later. The stamp also + matches how `OSDelta` carries `externalId`, which keeps a Delta and the Request built from it judged + the same way. nil means anonymous, including for caches written before ownership was stamped. diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift index a29434e21..8878c293b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift @@ -33,7 +33,7 @@ import OneSignalUserMocks @testable import OneSignalUser /// Covers the JWT bearer token on `OSIdentityModel`: which tokens count as usable, the -/// compare-and-set on invalidation, and what survives an archive round trip. +/// compare-and-set on invalidation, and what survives an archive round trip. Also what `clearData` keeps. final class OSIdentityModelTests: XCTestCase { override func setUpWithError() throws { @@ -59,6 +59,23 @@ final class OSIdentityModelTests: XCTestCase { return try XCTUnwrap(unarchiver.decodeObject(forKey: NSKeyedArchiveRootObjectKey) as? OSIdentityModel) } + // MARK: - clearData() + + /// The fetch that follows a clear is by `onesignal_id`, so it cannot change who the user is, and work + /// built before its response must not read this user as anonymous. + func testClearDataKeepsTheExternalIdAndDropsEveryOtherAlias() { + let model = OSIdentityModel( + aliases: [OS_ONESIGNAL_ID: userA_OSID, OS_EXTERNAL_ID: userA_EUID, "stale_label": "stale_value"], + changeNotifier: OSEventProducer() + ) + + model.clearData() + + XCTAssertEqual(model.externalId, userA_EUID) + XCTAssertNil(model.onesignalId) + XCTAssertNil(model.aliases["stale_label"]) + } + // MARK: - getValidJwt() func testGetValidJwtReturnsNilWhenTokenIsNil() { From 12be9d852622e847091d76758ddd18d591b1129c Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 11 Sep 2026 13:56:10 -0700 Subject: [PATCH 05/13] test: [SDK-5137] read the Delta queue through a snapshot in the user tests `OSOperationRepoTestSupport.snapshotDeltaQueue()` reads `deltaQueue` on the repo's own queue, but it lived in OneSignalOSCoreTests, so DeltaOwnershipTests and UserJwtLifecycleTests read the array directly from the test thread while the repo appends to it on `dispatchQueue`. Move the helper into OneSignalOSCoreMocks, which both test targets link, and route the two reads through it. No remove or update coverage is added, per the decision on #1710: neither Delta's owner reaches the server. --- .../OneSignalOSCoreMocks/OSCoreMocks.swift | 8 ++++++++ .../OSOperationRepoFlushTests.swift | 1 + .../OSOperationRepoIdentityVerificationTests.swift | 1 + .../OSOperationRepoTestSupport.swift | 10 ---------- .../OneSignalUserTests/DeltaOwnershipTests.swift | 3 ++- .../OneSignalUserTests/UserJwtLifecycleTests.swift | 2 +- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/OSCoreMocks.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/OSCoreMocks.swift index 4dae89435..31a65c3ea 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/OSCoreMocks.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/OSCoreMocks.swift @@ -55,4 +55,12 @@ extension OSOperationRepo { } paused = false } + + /** + The queue as of right now. Tests poll it while the repo appends on its own queue, so reading + `deltaQueue` directly is a data race even when only the count is wanted. + */ + public func snapshotDeltaQueue() -> [OSDelta] { + return dispatchQueue.sync { deltaQueue } + } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoFlushTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoFlushTests.swift index 2e01ca899..52cc1c79d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoFlushTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoFlushTests.swift @@ -29,6 +29,7 @@ import Foundation import XCTest import OneSignalCore import OneSignalCoreMocks +import OneSignalOSCoreMocks @testable import OneSignalOSCore /// Covers `flushDeltaQueue` routing: matched deltas go to executors and leave the repo diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoIdentityVerificationTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoIdentityVerificationTests.swift index 47ad71e0d..7b3e405c3 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoIdentityVerificationTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoIdentityVerificationTests.swift @@ -29,6 +29,7 @@ import Foundation import XCTest import OneSignalCore import OneSignalCoreMocks +import OneSignalOSCoreMocks import OneSignalKMP @testable import OneSignalOSCore diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoTestSupport.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoTestSupport.swift index fc84a9fba..bb84d4ec3 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoTestSupport.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSOperationRepoTestSupport.swift @@ -71,16 +71,6 @@ enum OSOperationRepoTestEnvironment { } } -extension OSOperationRepo { - /** - The queue as of right now. Tests poll it while the repo appends on its own queue, so reading - `deltaQueue` directly is a data race even when only the count is wanted. - */ - func snapshotDeltaQueue() -> [OSDelta] { - return dispatchQueue.sync { deltaQueue } - } -} - /// Records what the Operation Repo hands it, so tests can assert on routing rather than on requests. final class MockOperationExecutor: OSOperationExecutor { let supportedDeltas: [String] diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/DeltaOwnershipTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/DeltaOwnershipTests.swift index f798e56ee..f4c901715 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/DeltaOwnershipTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/DeltaOwnershipTests.swift @@ -28,6 +28,7 @@ import XCTest import OneSignalCore import OneSignalCoreMocks +import OneSignalOSCoreMocks import OneSignalUserMocks @testable import OneSignalOSCore @testable import OneSignalUser @@ -298,6 +299,6 @@ final class DeltaOwnershipTests: XCTestCase { } private func queuedDelta(named name: String, property: String) -> OSDelta? { - return OneSignalUserManagerImpl.sharedInstance.operationRepo.deltaQueue.first { $0.name == name && $0.property == property } + return OneSignalUserManagerImpl.sharedInstance.operationRepo.snapshotDeltaQueue().first { $0.name == name && $0.property == property } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserJwtLifecycleTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserJwtLifecycleTests.swift index 58514b954..01401ee1b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserJwtLifecycleTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserJwtLifecycleTests.swift @@ -109,7 +109,7 @@ final class UserJwtLifecycleTests: XCTestCase { /// The Delta `logout()` produces by silencing the push subscription. private func silencingDelta() -> OSDelta? { - return OneSignalUserManagerImpl.sharedInstance.operationRepo.deltaQueue.first { + return OneSignalUserManagerImpl.sharedInstance.operationRepo.snapshotDeltaQueue().first { $0.name == OS_UPDATE_SUBSCRIPTION_DELTA && $0.property == "isDisabledInternally" } } From 0b5fc76abac5262f7c9f503847abd8ad1fab6bee Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 11 Sep 2026 13:56:10 -0700 Subject: [PATCH 06/13] fix: [SDK-5137] send a Delete Subscription unsigned instead of parking it for a token `OSRequestDeleteSubscription.prepareForExecution` ran the Request through `authorize`, so under Identity Verification a delete whose owner had no valid token was parked and the app was asked for one, and the executor's failure handler treated a 401 as a rejected token. The route the SDK uses, `DELETE /apps/{app}/subscriptions/{id}`, performs no JWT check: the server registers it with auth skipped, only the by-alias variant validates Identity Verification claims, and the OAuth layer in front treats the user bearer as not applicable. Signing bought nothing, and parking cost an unsubscribe: `removeEmail` or `removeSms` while the user had no token, followed by a logout or a switch, left the delete parked, restored on every launch, asking for a token the app could no longer supply, and the subscription stayed on the server. Drop the `authorize` call and the 401 branch. The owner stays on the Request for the anonymous purge, which is what it was for. The ownership convention in OSUserRequest.swift records the exception next to Update Subscription, which sends unsigned for the same reason. Test in ExecutorAnonymousPurgeTests: with Identity Verification on, a delete owned by a user with no token goes out with no Authorization header and nobody is asked. Fails without the fix. --- .../OSSubscriptionOperationExecutor.swift | 5 ++--- .../OSRequestDeleteSubscription.swift | 10 +++++++--- .../Source/Requests/OSUserRequest.swift | 4 +++- .../ExecutorAnonymousPurgeTests.swift | 20 +++++++++++++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift index 43d3f63b9..1d14039bd 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift @@ -447,9 +447,8 @@ extension OSSubscriptionOperationExecutor { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor delete subscription request failed with error: \(error.debugDescription)") self.dispatchQueue.async { let responseType = OSNetworkingUtils.getResponseStatusType(error.code) - if responseType == .unauthorized, self.auth.handleUnauthorized(request) { - OneSignalLog.onesignalLog(.LL_DEBUG, message: "OSSubscriptionOperationExecutor holding \(request) for a new token") - } else if responseType != .retryable { + // No token handling: the delete is never signed, so a 401 here is not about the user's JWT. + if responseType != .retryable { // Fail, no retry, remove from cache and queue // If this request returns a missing status, that is ok as this is a delete request self.removeRequestQueue.removeAll(where: { $0 == request}) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift index cf162e809..9ceb4c2e0 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift @@ -46,15 +46,19 @@ class OSRequestDeleteSubscription: OneSignalRequest, OSUserRequest { See the ownership convention in `OSUserRequest.swift`. Removing an email or SMS subscription is a deliberate action on one user, so an anonymous one is dropped under Identity Verification even though the path addresses a subscription rather than a user. + + The owner serves that purge only. The endpoint is addressed by subscription ID and takes no user + JWT, so this Request is never signed and never parked for a token: parking it would hold an + unsubscribe on a credential the server does not read, and after a logout on one the app can no + longer supply. */ let ownerExternalId: String? - // Need the subscription_id + // Need the subscription_id. Not authorized: see `ownerExternalId`. func prepareForExecution(newRecordsState: OSNewRecordsState, auth: OSRequestAuthorizing) -> Bool { if let subscriptionId = subscriptionModel.subscriptionId, newRecordsState.canAccess(subscriptionId), - let appId = OneSignalIdentifiers.currentAppId, - auth.authorize(self) + let appId = OneSignalIdentifiers.currentAppId { self.path = "apps/\(appId)/subscriptions/\(subscriptionId)" return true diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index b2a26a5ee..92471d585 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -69,7 +69,9 @@ extension OSUserRequest { Three Requests are nil by construction and so are never signed: Identify User and Fetch Identity By Subscription both address a user that has no `external_id` yet, and Update Subscription is the - device's own push subscription. Each says why at its declaration. + device's own push subscription. Delete Subscription carries an owner for the purge but is never signed + either, since its endpoint is addressed by subscription ID and takes no user JWT. Each says why at its + declaration. */ internal extension OneSignalRequest { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorAnonymousPurgeTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorAnonymousPurgeTests.swift index ff103ab63..6509b847b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorAnonymousPurgeTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorAnonymousPurgeTests.swift @@ -349,6 +349,26 @@ final class ExecutorAnonymousPurgeTests: XCTestCase { XCTAssertEqual(cachedRequestOwners(OS_SUBSCRIPTION_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY, of: OSRequestDeleteSubscription.self), [userA_EUID]) } + /// The delete endpoint is addressed by subscription ID and takes no user JWT, so an owned delete goes + /// out unsigned even when its owner has no token, and nobody is asked for one. Parking it would hold + /// an unsubscribe on a credential the server does not read. + func testTheSubscriptionExecutorSendsAnOwnedDeleteUnsignedWithoutAskingForAToken() { + let tokenless = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userB_OSID, OS_EXTERNAL_ID: userB_EUID], changeNotifier: OSEventProducer()) + OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(tokenless) + let executor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState, auth: auth) + + let removal = subscriptionDelta(OS_REMOVE_SUBSCRIPTION_DELTA, for: tokenless, subscription: subscription(id: "tokenless-subscription-id")) + executor.enqueueDelta(removal) + executor.processDeltaQueue(inBackground: false) + OneSignalCoreMocks.waitUntil("The owned delete was not sent") { + self.client.hasExecutedRequestOfType(OSRequestDeleteSubscription.self, expectedCount: 1) + } + + XCTAssertNil(client.executedRequests.first?.additionalHeaders?["Authorization"]) + XCTAssertTrue(OneSignalUserManagerImpl.sharedInstance.userJwtRepo.pendingTokenAsks().isEmpty, + "nobody may be asked for a token the endpoint does not read") + } + /// An Update Subscription is addressed by subscription ID and never signed, so it has no owner to be /// judged by and the purge has to leave that queue alone: `logout()`'s unsubscribe travels in it. func testTheSubscriptionExecutorKeepsEveryUpdateRequest() { From bb21d206433e20e1284d7f2faef0e09a7f155e41 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 16 Sep 2026 16:57:36 -0700 Subject: [PATCH 07/13] fix: [SDK-5137] remove a Request from the queue only after its response hydrates the id `dropIdentifyUsersThatCanNeverPrepare` runs on every send and drops an Identify User whose user has no `onesignal_id` and no queued Create User or Fetch Identity By Subscription to supply one. The response handlers that supply an id dispatched `removeFromQueue` first and hydrated the model after it, on the callback thread. A send pass landing on the executor queue between the two saw no id and no supplier, dropped the Identify User and persisted the drop. Narrow, since the pass has to be dispatched inside that gap by remote params hydrating, a session start, a login, a token update or a delayed retry, but the loss is the offline login the restored `[Create User, Identify User]` archive exists to keep. Dispatch the removal after the hydrate at the three sites that supply an id (Create User, Fetch Identity By Subscription, Identify User). A pass that runs before the removal still sees the supplier queued; one that runs after it is ordered behind the hydrate by the removal's dispatch. The drop itself stays per send, since it also ends the livelock `main` had when a Fetch Identity By Subscription failed for good. The ordering is documented on the drop rule and at each site; no deterministic test can pin it without a hook inside the response handler, so none is added. Found by the adversarial review of #1740. --- .../Source/Executors/OSUserExecutor.swift | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 0c7c05e1d..f35187056 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -127,9 +127,16 @@ class OSUserExecutor { /** With the requirement off, an Identify User whose user never received an `onesignal_id`, and has no queued Create User or Fetch Identity By Subscription left to supply one, can never prepare. Drop it - rather than let it hold the queue and block the logins behind it. Only reachable for a login kept at - start while the requirement was still unknown; `uncacheUserRequests` drops the same Request when the - requirement is already known to be off. + rather than let it hold the queue and block the logins behind it. Reached by a login kept at start + while the requirement was still unknown (`uncacheUserRequests` drops that Request when the + requirement is already known to be off) and by one whose Fetch Identity By Subscription failed for + good. + + Runs on every send, so it relies on each response handler removing its Request from the queue only + after it has hydrated the `onesignal_id` it supplies. A pass that runs before that removal still + sees the supplier queued; one that runs after it is ordered behind the hydrate by the removal's + dispatch. Removing first would let a pass in between drop, and persist the drop of, a login whose + id was a few instructions away. */ private func dropIdentifyUsersThatCanNeverPrepare() { guard identityVerificationService.requirement == .off else { @@ -388,8 +395,6 @@ extension OSUserExecutor { request.sentToClient = true OneSignalCoreImpl.sharedClient().execute(request) { response in - self.removeFromQueue(request) - // Create User's response won't send us the user's complete info if this user already exists if let response = response { // Parse the response for any data we need to update @@ -399,6 +404,10 @@ extension OSUserExecutor { originalPushToken: request.originalPushToken, addNewRecords: request.addsNewRecords ) + // Only now, with the `onesignal_id` hydrated. An Identify User queued behind this Create + // User reads that id, and once the Create User has left the queue nothing else tells + // `dropIdentifyUsersThatCanNeverPrepare` that an id is on its way. + self.removeFromQueue(request) // If this user already exists and we logged into an external_id, fetch the user data // Fetch the user only if its the current user and non-anonymous @@ -425,6 +434,8 @@ extension OSUserExecutor { OSConsistencyManager.shared.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: onesignalId) } } + } else { + self.removeFromQueue(request) } OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = false } onFailure: { error in @@ -471,12 +482,12 @@ extension OSUserExecutor { request.sentToClient = true OneSignalCoreImpl.sharedClient().execute(request) { response in - self.removeFromQueue(request) - if let identityObject = self.parseIdentityObjectResponse(response), let onesignalId = identityObject[OS_ONESIGNAL_ID] { request.identityModel.hydrate(identityObject) OSUserStateSnapshot.fireUserStateChangedIfCurrent(request.identityModel) + // After the hydrate, for the Identify User queued behind this Request; see `executeCreateUserRequest`. + self.removeFromQueue(request) // Fetch this user's data if it is the current user guard OneSignalUserManagerImpl.sharedInstance.currentUser(matching: request.identityModel.modelId) != nil @@ -486,6 +497,8 @@ extension OSUserExecutor { } self.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModel) + } else { + self.removeFromQueue(request) } } onFailure: { error in OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor executeFetchIdentityBySubscriptionRequest failed with error: \(error.debugDescription)") @@ -525,10 +538,9 @@ extension OSUserExecutor { request.sentToClient = true OneSignalCoreImpl.sharedClient().execute(request) { _ in - self.removeFromQueue(request) - guard let onesignalId = request.identityModelToIdentify.onesignalId else { OneSignalLog.onesignalLog(.LL_ERROR, message: "executeIdentifyUserRequest succeeded but is now missing OneSignal ID!") + self.removeFromQueue(request) self.executePendingRequests() return } @@ -540,6 +552,8 @@ extension OSUserExecutor { ] request.identityModelToUpdate.hydrate(aliases) OSUserStateSnapshot.fireUserStateChangedIfCurrent(request.identityModelToUpdate) + // After the hydrate, like every handler that supplies an `onesignal_id`; see `executeCreateUserRequest`. + self.removeFromQueue(request) // the anonymous user has been identified, still need to Fetch User as we cleared local data if OneSignalUserManagerImpl.sharedInstance.currentUser(matching: request.identityModelToUpdate.modelId) != nil { From 0c64963a69e735fe1bccdca69b86c648b804b2ef Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 16 Sep 2026 16:57:36 -0700 Subject: [PATCH 08/13] fix: [SDK-5137] decide the unsigned Delete Subscription inside OSRequestAuth 0b5fc76ab stopped signing Delete Subscription by skipping `authorize` altogether, while `sendsUnsigned` still read as if only Update Subscription went out with no header. Two mechanisms for one exemption, and the header doc's claim that `OSRequestAuth` is the one place a header is decided no longer held. Have `authorize` let any `sendsUnsigned` Request through unsigned up front, owner or not, and have Delete Subscription declare the flag and call `authorize` again from its prepare. Update Subscription is unaffected: its owner is always nil, so it took the exempt branch either way. The doc on `sendsUnsigned` now names both Requests, and the ownership convention says Delete declares the flag rather than that it is "never signed either". Tests in OSRequestAuthTests: an owned exempt Request with no token goes through unsigned, unparked and without an ask, and one with a token on hand is still not signed. Both fail against the previous commit's `authorize`, as does the purge test's unsigned delete, since the Delete prepare calls `authorize` again. Found by the adversarial review of #1740. --- .../OneSignalUser/Source/OSRequestAuth.swift | 12 +++++----- .../OSRequestDeleteSubscription.swift | 20 ++++++++++------- .../Source/Requests/OSUserRequest.swift | 13 ++++++----- .../OSRequestAuthTests.swift | 22 +++++++++++++++++++ 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift index 835c0258e..6993a8ce3 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift @@ -53,7 +53,7 @@ protocol OSRequestAuthorizing: AnyObject { /// The same decision for endpoints that take a token but no alias, because their path names a /// subscription or the app. Returns `false` under the same conditions as `authorizeUserScoped`, - /// except that a `sendsUnsigned` Request with no owner is allowed through. + /// except that a `sendsUnsigned` Request always goes through with no header, owner or not. func authorize(_ request: OSUserRequest) -> Bool /** @@ -161,16 +161,14 @@ final class OSRequestAuth: OSRequestAuthorizing { func authorize(_ request: OSUserRequest) -> Bool { forgetPark(of: request) - guard ivBehaviorActive else { + // An exempt Request is never signed, so an owner it keeps for the purge is not looked at here. + guard ivBehaviorActive, !request.sendsUnsigned else { return true } guard let externalId = request.ownerExternalId else { // Anything not exempt is a leftover the purge has yet to clear, and unsendable until it does. - guard request.sendsUnsigned else { - OneSignalLog.onesignalLog(.LL_ERROR, message: "OSRequestAuth: refusing \(request), it has no owner under Identity Verification") - return false - } - return true + OneSignalLog.onesignalLog(.LL_ERROR, message: "OSRequestAuth: refusing \(request), it has no owner under Identity Verification") + return false } guard let token = jwt.validJwt(externalId: externalId) else { park(request, ownedBy: externalId) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift index 9ceb4c2e0..664b1a797 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift @@ -45,20 +45,24 @@ class OSRequestDeleteSubscription: OneSignalRequest, OSUserRequest { /** See the ownership convention in `OSUserRequest.swift`. Removing an email or SMS subscription is a deliberate action on one user, so an anonymous one is dropped under Identity Verification even - though the path addresses a subscription rather than a user. - - The owner serves that purge only. The endpoint is addressed by subscription ID and takes no user - JWT, so this Request is never signed and never parked for a token: parking it would hold an - unsubscribe on a credential the server does not read, and after a logout on one the app can no - longer supply. + though the path addresses a subscription rather than a user. The owner serves that purge only; + see `sendsUnsigned`. */ let ownerExternalId: String? - // Need the subscription_id. Not authorized: see `ownerExternalId`. + /** + The endpoint is addressed by subscription ID and takes no user JWT, so this Request is never signed + and never parked for a token: parking it would hold an unsubscribe on a credential the server does + not read, and after a logout on one the app can no longer supply. + */ + var sendsUnsigned: Bool { return true } + + // Need the subscription_id func prepareForExecution(newRecordsState: OSNewRecordsState, auth: OSRequestAuthorizing) -> Bool { if let subscriptionId = subscriptionModel.subscriptionId, newRecordsState.canAccess(subscriptionId), - let appId = OneSignalIdentifiers.currentAppId + let appId = OneSignalIdentifiers.currentAppId, + auth.authorize(self) { self.path = "apps/\(appId)/subscriptions/\(subscriptionId)" return true diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index 92471d585..92af8f36f 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -34,9 +34,10 @@ protocol OSUserRequest: OneSignalRequest, NSCoding { /// The user this Request belongs to; also selects its token. See the ownership convention below. var ownerExternalId: String? { get } - /// Whether this Request may still be sent with no `Authorization` header once Identity Verification - /// is in effect. Only Update Subscription may: its path names a subscription rather than a user, so - /// there is no user for the server to authorize. Everything else with no owner is refused. + /// Whether this Request goes out with no `Authorization` header even once Identity Verification is + /// in effect. Two do, because their paths name a subscription rather than a user and their endpoints + /// take no user JWT: Update Subscription, which has no owner either, and Delete Subscription, which + /// keeps its owner for the purge. Everything else with no owner is refused. var sendsUnsigned: Bool { get } /// Builds the path and resolves authorization. `false` leaves the Request queued, whether it is @@ -69,9 +70,9 @@ extension OSUserRequest { Three Requests are nil by construction and so are never signed: Identify User and Fetch Identity By Subscription both address a user that has no `external_id` yet, and Update Subscription is the - device's own push subscription. Delete Subscription carries an owner for the purge but is never signed - either, since its endpoint is addressed by subscription ID and takes no user JWT. Each says why at its - declaration. + device's own push subscription. Delete Subscription carries an owner for the purge but declares + `sendsUnsigned`, since its endpoint is addressed by subscription ID and takes no user JWT. Each says + why at its declaration. */ internal extension OneSignalRequest { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift index c91107deb..c376e5da8 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift @@ -238,6 +238,28 @@ final class OSRequestAuthTests: XCTestCase { XCTAssertNil(request.authorizationHeader) } + /// The subscription delete keeps its owner for the purge and still goes out unsigned: the endpoint + /// takes no user JWT, so a missing token must neither park it nor ask the app for one. + func testAuthorizeSendsAnOwnedExemptRequestUnsignedWithoutAsking() { + let auth = makeAuth(requiresUserAuth: true) + let request = StubUserRequest(ownerExternalId: "user-a", sendsUnsigned: true) + + XCTAssertTrue(auth.authorize(request)) + XCTAssertNil(request.authorizationHeader) + XCTAssertTrue(jwt.askedFor.isEmpty) + XCTAssertFalse(auth.parkedForToken(request)) + } + + /// A token on hand changes nothing: the header would be ignored, and a rejection could not be about it. + func testAuthorizeDoesNotSignAnOwnedExemptRequestThatHasAToken() { + let auth = makeAuth(requiresUserAuth: true) + jwt.tokens["user-a"] = "token-a" + let request = StubUserRequest(ownerExternalId: "user-a", sendsUnsigned: true) + + XCTAssertTrue(auth.authorize(request)) + XCTAssertNil(request.authorizationHeader) + } + /// Everything else with no owner is a leftover the purge has yet to clear, and must not go out unsigned. func testAuthorizeRefusesAnUnownedRequestThatIsNotExempt() { let auth = makeAuth(requiresUserAuth: true) From 53b1740bddee7a56b30c8b803d264a13bdbc669c Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 16 Sep 2026 16:57:36 -0700 Subject: [PATCH 09/13] test: [SDK-5137] cover the Identify User and Fetch Identity reporting sites 1d1f05039 moved the user-state report to three executor sites but tested only the parked Create User. The other two are reachable with Identity Verification off: a `login` while anonymous followed by a second `login` before the Identify User returns reported the first user after the app had switched to the second, and a 3.x fetch-identity landing after a `login` reported the anonymous user the app had logged in over. Both new tests fail against an unconditional fire at their site. A deleted call would not fail them, since the Fetch User that follows re-reports the current user; the guard is what they pin. Also from the review of that file: the idle check read the mock client's request lists off the test thread while the mock appends under its lock, so the mock gains a lock-backed `isIdle` and the test observer keeps its states under a lock. The negative count assertion does not depend on the 0.1s pause after it, since the report decision is made inside the response block and the mock records completion after that block returns; the comment now says so, and the pause stays to drain the executor queue before teardown. --- .../MockOneSignalClient.swift | 6 + .../UserStateReportingTests.swift | 119 +++++++++++++++--- 2 files changed, 111 insertions(+), 14 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift b/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift index 1b5288797..292e50e10 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift @@ -283,4 +283,10 @@ extension MockOneSignalClient { request.isKind(of: type) }.count } + + /// Whether every request that entered `execute` has also completed, read under the lock the two + /// lists are written with. A held request counts as started and not completed. + public var isIdle: Bool { + return lock.withLock { startedRequests.count == completedRequests.count } + } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift index 0d8347990..c92e63fab 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift @@ -34,16 +34,23 @@ import OneSignalUserMocks @testable import OneSignalUser private class MockUserStateObserver: NSObject, OSUserStateObserver { - var states: [OSUserState] = [] + private let lock = NSLock() + private var reported: [OSUserState] = [] + + /// Read on the test thread while the SDK reports from its response threads. + var states: [OSUserState] { + return lock.withLock { reported } + } func onUserStateDidChange(state: OSUserChangedState) { - states.append(state.current) + lock.withLock { reported.append(state.current) } } } /** What the app's `OSUserStateObserver` hears, and what the persisted snapshot names, once a Request can - complete for a user who is no longer current. + complete for a user who is no longer current. One case per executor site that hydrates an identity + model: Create User, Identify User, and Fetch Identity By Subscription. */ final class UserStateReportingTests: XCTestCase { private var client = MockOneSignalClient() @@ -66,7 +73,7 @@ final class UserStateReportingTests: XCTestCase { override func tearDownWithError() throws { // A Request still in flight would land mid-next-test and hydrate the shared models under it. - OneSignalCoreMocks.waitUntil("A Request was still in flight at teardown") { self.clientIsIdle } + OneSignalCoreMocks.waitUntil("A Request was still in flight at teardown") { self.client.isIdle } OneSignalUserManagerImpl.sharedInstance.removeObserver(observer) OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = false OneSignalCoreMocks.clearUserDefaults() @@ -95,7 +102,10 @@ final class UserStateReportingTests: XCTestCase { OneSignalCoreMocks.waitUntil("A's parked Create User was not sent") { self.client.executedRequests.contains { ($0 as? OSRequestCreateUser)?.identityModel.externalId == userA_EUID } } - OneSignalCoreMocks.waitUntil("A's Create User was still in flight") { self.clientIsIdle } + // Settled once idle: the report decision is made inside the response block, and the mock records + // a request as completed only after that block returns. The pause just lets the executor queue + // drain what the response dispatched before teardown. + OneSignalCoreMocks.waitUntil("A's Create User was still in flight") { self.client.isIdle } allowAsyncWorkToRun(seconds: 0.1) // Hydrated, so anything queued for A has its onesignal_id. @@ -103,28 +113,109 @@ final class UserStateReportingTests: XCTestCase { // Not reported: the app hears nothing new, and the persisted pair still names B. XCTAssertEqual(observer.states.count, reportsBeforeA, "the app must not hear about A: \(observer.states)") XCTAssertEqual(observer.states.last?.externalId, userB_EUID) - XCTAssertEqual(OneSignalUserDefaults.initShared().getSavedString(forKey: OS_SNAPSHOT_EXTERNAL_ID, defaultValue: nil), userB_EUID) - XCTAssertEqual(OneSignalUserDefaults.initShared().getSavedString(forKey: OS_SNAPSHOT_ONESIGNAL_ID, defaultValue: nil), userB_OSID) + assertPersistedSnapshotNames(externalId: userB_EUID, onesignalId: userB_OSID) + } + + /** + Needs no Identity Verification. A `login` while anonymous identifies that user, and a second `login` + before the response lands makes another user current. The Identify User still hydrates the first + user's model, since Requests queued behind it read the `onesignal_id`, but the app must not hear a + user it has already switched away from. + */ + func testAnIdentifyUserThatCompletesAfterAUserSwitchDoesNotReportThatUser() { + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: false) + MockUserRequests.setDefaultCreateAnonUserResponses(with: client) + MockUserRequests.setDefaultIdentifyUserResponses(with: client, externalId: userA_EUID) + // The anonymous user needs its onesignal_id first, or the Identify User cannot address it. + OneSignalUserManagerImpl.sharedInstance.start() + OneSignalCoreMocks.waitUntil("The anonymous user was not created") { + OneSignalUserManagerImpl.sharedInstance.user.identityModel.onesignalId == anonUserOSID && self.client.isIdle + } + + client.holdResponses = true + OneSignalUserManagerImpl.sharedInstance.login(externalId: userA_EUID, token: nil) + OneSignalCoreMocks.waitUntil("The Identify User was not started") { + self.client.startedRequestCount(ofType: OSRequestIdentifyUser.self) == 1 + } + // Makes B current while A's Identify User is still in flight; B's Create User queues behind it. + OneSignalUserManagerImpl.sharedInstance.login(externalId: userB_EUID, token: nil) + client.releaseHeldResponses() + waitForTheLoginToSettle() + + // Hydrated, so anything queued for A has its onesignal_id. + XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance.identityModelRepo.get(externalId: userA_EUID)?.onesignalId, anonUserOSID) + XCTAssertFalse(observer.states.contains { $0.externalId == userA_EUID }, "the app must not hear about A: \(observer.states)") + XCTAssertEqual(observer.states.last?.externalId, userB_EUID) + assertPersistedSnapshotNames(externalId: userB_EUID, onesignalId: userB_OSID) + } + + /** + The 3.x upgrade path, again with no Identity Verification. The fetch identifies an anonymous user, + and a `login` that lands before its response makes an identified user current. The fetch still + hydrates the anonymous model, which the Identify User queued behind it needs, but the app must not + hear an anonymous user it has already logged in over. + */ + func testAFetchIdentityBySubscriptionThatCompletesAfterALoginDoesNotReportTheAnonymousUser() { + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: false) + let legacyPlayerId = "legacy_player_id" + let legacyOnesignalId = "legacy_player_onesignal_id" + OneSignalUserDefaults.initShared().saveString(forKey: OSUD_LEGACY_PLAYER_ID, withValue: legacyPlayerId) + client.setMockResponseForRequest( + request: "OSRequestFetchIdentityBySubscription with subscriptionId: \(legacyPlayerId)", + response: MockUserRequests.testIdentityPayload(onesignalId: legacyOnesignalId, externalId: nil) + ) + client.setMockResponseForRequest( + request: "", + response: MockUserRequests.testIdentityPayload(onesignalId: legacyOnesignalId, externalId: userA_EUID) + ) + client.setMockResponseForRequest( + request: "", + response: MockUserRequests.testIdentityPayload(onesignalId: legacyOnesignalId, externalId: userA_EUID) + ) + client.holdResponses = true + + // Migrates the legacy player into an anonymous user whose identity the held fetch supplies. + OneSignalUserManagerImpl.sharedInstance.start() + let anonymousModel = OneSignalUserManagerImpl.sharedInstance.user.identityModel + XCTAssertNil(anonymousModel.onesignalId) + OneSignalCoreMocks.waitUntil("The Fetch Identity By Subscription was not started") { + self.client.startedRequestCount(ofType: OSRequestFetchIdentityBySubscription.self) == 1 + } + // Makes A current while the anonymous user's fetch is still in flight; A's Identify User queues behind it. + OneSignalUserManagerImpl.sharedInstance.login(externalId: userA_EUID, token: nil) + client.releaseHeldResponses() + waitForTheLoginToSettle() + + // Hydrated, so the Identify User behind the fetch could address the anonymous user. + XCTAssertEqual(anonymousModel.onesignalId, legacyOnesignalId) + XCTAssertFalse(observer.states.contains { $0.externalId == nil }, "the app must not hear the anonymous user: \(observer.states)") + XCTAssertEqual(observer.states.last?.externalId, userA_EUID) + assertPersistedSnapshotNames(externalId: userA_EUID, onesignalId: legacyOnesignalId) } - // MARK: - Waits + // MARK: - Helpers - private var clientIsIdle: Bool { - return client.completedRequests.count == client.startedRequests.count + private func assertPersistedSnapshotNames( + externalId: String, onesignalId: String, file: StaticString = #filePath, line: UInt = #line + ) { + let defaults = OneSignalUserDefaults.initShared() + XCTAssertEqual(defaults.getSavedString(forKey: OS_SNAPSHOT_EXTERNAL_ID, defaultValue: nil), externalId, file: file, line: line) + XCTAssertEqual(defaults.getSavedString(forKey: OS_SNAPSHOT_ONESIGNAL_ID, defaultValue: nil), onesignalId, file: file, line: line) } /** - Outlasts every effect of an identified login, including the Fetch User its Create User starts. A - fetch landing later would re-report the current user and hide a wrong report made in between. + Outlasts every effect of an identified login, including the Fetch User its Create User or Identify + User starts. A fetch landing later would re-report the current user and hide a wrong report made in + between. */ private func waitForTheLoginToSettle() { OneSignalCoreMocks.waitUntil("The login did not reach a reported user") { OneSignalUserManagerImpl.sharedInstance.user.identityModel.onesignalId != nil && self.observer.states.contains { $0.onesignalId != nil } && self.client.hasCompletedRequestOfType(OSRequestFetchUser.self) - && self.clientIsIdle + && self.client.isIdle } allowAsyncWorkToRun(seconds: 0.1) - OneSignalCoreMocks.waitUntil("The login left a Request in flight") { self.clientIsIdle } + OneSignalCoreMocks.waitUntil("The login left a Request in flight") { self.client.isIdle } } } From 35fc6d94ae16515a0ae6adf7c95a0ae82050f678 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 16 Sep 2026 16:57:36 -0700 Subject: [PATCH 10/13] test: [SDK-5137] pin that a fetch response merges into the kept external_id d9d6f1105 keeps `external_id` across `clearData` and accepted that a Fetch User response omitting it no longer demotes the user, but the test stopped at the clear. What makes the kept alias safe is `internalAddAliases` merging rather than replacing the dictionary, and nothing locked that in. Add the hydrate step: after a clear, a response carrying only `onesignal_id` leaves `external_id` in place. --- .../OSIdentityModelTests.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift index 8878c293b..3e65611c6 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift @@ -76,6 +76,22 @@ final class OSIdentityModelTests: XCTestCase { XCTAssertNil(model.aliases["stale_label"]) } + /// The response to that fetch merges into what the clear kept, which is what keeping `external_id` + /// relies on. A response that omits it, which only a server-side unlink produces, therefore leaves + /// the user identified rather than demoting it; the next `login` corrects that. + func testHydrateAfterClearDataMergesIntoTheKeptExternalId() { + let model = OSIdentityModel( + aliases: [OS_ONESIGNAL_ID: userA_OSID, OS_EXTERNAL_ID: userA_EUID], + changeNotifier: OSEventProducer() + ) + model.clearData() + + model.hydrate([OS_ONESIGNAL_ID: userA_OSID]) + + XCTAssertEqual(model.onesignalId, userA_OSID) + XCTAssertEqual(model.externalId, userA_EUID) + } + // MARK: - getValidJwt() func testGetValidJwtReturnsNilWhenTokenIsNil() { From 5ca8f920e19baf952671e8a0353363f1afd320f5 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 16 Sep 2026 19:38:08 -0700 Subject: [PATCH 11/13] style: [SDK-5137] shorten the comments added by the review follow-ups Comment-only. Each hydrate-then-remove site is down to one line that points at `dropIdentifyUsersThatCanNeverPrepare`, which states the constraint once. The test docs name the scenario in a sentence or two instead of retelling it, and the rest drop wording that contrasted with the code they replaced. --- .../MockOneSignalClient.swift | 3 +- .../Source/Executors/OSUserExecutor.swift | 24 ++++-------- .../OneSignalUser/Source/OSRequestAuth.swift | 2 +- .../OSRequestDeleteSubscription.swift | 6 +-- .../Source/Requests/OSUserRequest.swift | 7 ++-- .../OSIdentityModelTests.swift | 5 +-- .../OSRequestAuthTests.swift | 5 +-- .../UserStateReportingTests.swift | 38 +++++++------------ 8 files changed, 31 insertions(+), 59 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift b/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift index 292e50e10..1c865af66 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift @@ -284,8 +284,7 @@ extension MockOneSignalClient { }.count } - /// Whether every request that entered `execute` has also completed, read under the lock the two - /// lists are written with. A held request counts as started and not completed. + /// Held requests count as in flight. Read under the lock, so safe from the test thread. public var isIdle: Bool { return lock.withLock { startedRequests.count == completedRequests.count } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index f35187056..903c771f4 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -125,18 +125,10 @@ class OSUserExecutor { } /** - With the requirement off, an Identify User whose user never received an `onesignal_id`, and has no - queued Create User or Fetch Identity By Subscription left to supply one, can never prepare. Drop it - rather than let it hold the queue and block the logins behind it. Reached by a login kept at start - while the requirement was still unknown (`uncacheUserRequests` drops that Request when the - requirement is already known to be off) and by one whose Fetch Identity By Subscription failed for - good. - - Runs on every send, so it relies on each response handler removing its Request from the queue only - after it has hydrated the `onesignal_id` it supplies. A pass that runs before that removal still - sees the supplier queued; one that runs after it is ordered behind the hydrate by the removal's - dispatch. Removing first would let a pass in between drop, and persist the drop of, a login whose - id was a few instructions away. + An Identify User whose user has no `onesignal_id`, and no queued Create User or Fetch Identity By + Subscription to supply one, can never prepare, so drop it rather than let it block the logins behind it. + Runs on every send, so a handler that supplies an id must hydrate before it removes its Request from + the queue; a pass between the two would see no supplier and drop the login. */ private func dropIdentifyUsersThatCanNeverPrepare() { guard identityVerificationService.requirement == .off else { @@ -404,9 +396,7 @@ extension OSUserExecutor { originalPushToken: request.originalPushToken, addNewRecords: request.addsNewRecords ) - // Only now, with the `onesignal_id` hydrated. An Identify User queued behind this Create - // User reads that id, and once the Create User has left the queue nothing else tells - // `dropIdentifyUsersThatCanNeverPrepare` that an id is on its way. + // Must follow the hydrate; see `dropIdentifyUsersThatCanNeverPrepare`. self.removeFromQueue(request) // If this user already exists and we logged into an external_id, fetch the user data @@ -486,7 +476,7 @@ extension OSUserExecutor { let onesignalId = identityObject[OS_ONESIGNAL_ID] { request.identityModel.hydrate(identityObject) OSUserStateSnapshot.fireUserStateChangedIfCurrent(request.identityModel) - // After the hydrate, for the Identify User queued behind this Request; see `executeCreateUserRequest`. + // Must follow the hydrate; see `dropIdentifyUsersThatCanNeverPrepare`. self.removeFromQueue(request) // Fetch this user's data if it is the current user @@ -552,7 +542,7 @@ extension OSUserExecutor { ] request.identityModelToUpdate.hydrate(aliases) OSUserStateSnapshot.fireUserStateChangedIfCurrent(request.identityModelToUpdate) - // After the hydrate, like every handler that supplies an `onesignal_id`; see `executeCreateUserRequest`. + // Must follow the hydrate; see `dropIdentifyUsersThatCanNeverPrepare`. self.removeFromQueue(request) // the anonymous user has been identified, still need to Fetch User as we cleared local data diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift index 6993a8ce3..a774e548d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift @@ -161,7 +161,7 @@ final class OSRequestAuth: OSRequestAuthorizing { func authorize(_ request: OSUserRequest) -> Bool { forgetPark(of: request) - // An exempt Request is never signed, so an owner it keeps for the purge is not looked at here. + // An exempt Request may carry an owner for the purge; it still goes out unsigned. guard ivBehaviorActive, !request.sendsUnsigned else { return true } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift index 664b1a797..c67e0aa8e 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestDeleteSubscription.swift @@ -50,11 +50,7 @@ class OSRequestDeleteSubscription: OneSignalRequest, OSUserRequest { */ let ownerExternalId: String? - /** - The endpoint is addressed by subscription ID and takes no user JWT, so this Request is never signed - and never parked for a token: parking it would hold an unsubscribe on a credential the server does - not read, and after a logout on one the app can no longer supply. - */ + /// The endpoint takes no user JWT, so waiting for a token would only delay the unsubscribe, forever after a logout. var sendsUnsigned: Bool { return true } // Need the subscription_id diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index 92af8f36f..327e7a31a 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -34,10 +34,9 @@ protocol OSUserRequest: OneSignalRequest, NSCoding { /// The user this Request belongs to; also selects its token. See the ownership convention below. var ownerExternalId: String? { get } - /// Whether this Request goes out with no `Authorization` header even once Identity Verification is - /// in effect. Two do, because their paths name a subscription rather than a user and their endpoints - /// take no user JWT: Update Subscription, which has no owner either, and Delete Subscription, which - /// keeps its owner for the purge. Everything else with no owner is refused. + /// Whether this Request goes out with no `Authorization` header even under Identity Verification: + /// Update Subscription and Delete Subscription, whose endpoints are addressed by subscription ID and + /// take no user JWT. Everything else with no owner is refused. var sendsUnsigned: Bool { get } /// Builds the path and resolves authorization. `false` leaves the Request queued, whether it is diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift index 3e65611c6..0dc4f259c 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift @@ -76,9 +76,8 @@ final class OSIdentityModelTests: XCTestCase { XCTAssertNil(model.aliases["stale_label"]) } - /// The response to that fetch merges into what the clear kept, which is what keeping `external_id` - /// relies on. A response that omits it, which only a server-side unlink produces, therefore leaves - /// the user identified rather than demoting it; the next `login` corrects that. + /// The fetch response merges into what the clear kept, so one without `external_id` leaves the user + /// identified until the next `login`. func testHydrateAfterClearDataMergesIntoTheKeptExternalId() { let model = OSIdentityModel( aliases: [OS_ONESIGNAL_ID: userA_OSID, OS_EXTERNAL_ID: userA_EUID], diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift index c376e5da8..fe73e7e25 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSRequestAuthTests.swift @@ -238,8 +238,7 @@ final class OSRequestAuthTests: XCTestCase { XCTAssertNil(request.authorizationHeader) } - /// The subscription delete keeps its owner for the purge and still goes out unsigned: the endpoint - /// takes no user JWT, so a missing token must neither park it nor ask the app for one. + /// Delete Subscription keeps an owner for the purge; a missing token must not park it or ask the app. func testAuthorizeSendsAnOwnedExemptRequestUnsignedWithoutAsking() { let auth = makeAuth(requiresUserAuth: true) let request = StubUserRequest(ownerExternalId: "user-a", sendsUnsigned: true) @@ -250,7 +249,7 @@ final class OSRequestAuthTests: XCTestCase { XCTAssertFalse(auth.parkedForToken(request)) } - /// A token on hand changes nothing: the header would be ignored, and a rejection could not be about it. + /// Not signed even with a token on hand; the endpoint ignores the header. func testAuthorizeDoesNotSignAnOwnedExemptRequestThatHasAToken() { let auth = makeAuth(requiresUserAuth: true) jwt.tokens["user-a"] = "token-a" diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift index c92e63fab..1af0598be 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserStateReportingTests.swift @@ -48,9 +48,8 @@ private class MockUserStateObserver: NSObject, OSUserStateObserver { } /** - What the app's `OSUserStateObserver` hears, and what the persisted snapshot names, once a Request can - complete for a user who is no longer current. One case per executor site that hydrates an identity - model: Create User, Identify User, and Fetch Identity By Subscription. + What the app's `OSUserStateObserver` hears, and what the persisted snapshot names, once a Request + completes for a user who is no longer current. One test per executor site that hydrates an identity model. */ final class UserStateReportingTests: XCTestCase { private var client = MockOneSignalClient() @@ -102,10 +101,9 @@ final class UserStateReportingTests: XCTestCase { OneSignalCoreMocks.waitUntil("A's parked Create User was not sent") { self.client.executedRequests.contains { ($0 as? OSRequestCreateUser)?.identityModel.externalId == userA_EUID } } - // Settled once idle: the report decision is made inside the response block, and the mock records - // a request as completed only after that block returns. The pause just lets the executor queue - // drain what the response dispatched before teardown. + // The report fires inside the response block, before the mock counts it complete, so idle means decided. OneSignalCoreMocks.waitUntil("A's Create User was still in flight") { self.client.isIdle } + // Drains the executor queue before teardown. allowAsyncWorkToRun(seconds: 0.1) // Hydrated, so anything queued for A has its onesignal_id. @@ -116,17 +114,13 @@ final class UserStateReportingTests: XCTestCase { assertPersistedSnapshotNames(externalId: userB_EUID, onesignalId: userB_OSID) } - /** - Needs no Identity Verification. A `login` while anonymous identifies that user, and a second `login` - before the response lands makes another user current. The Identify User still hydrates the first - user's model, since Requests queued behind it read the `onesignal_id`, but the app must not hear a - user it has already switched away from. - */ + /// A second `login` before the first one's Identify User returns makes another user current. The + /// response still hydrates the first user's model, but the app must not hear that user. func testAnIdentifyUserThatCompletesAfterAUserSwitchDoesNotReportThatUser() { OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: false) MockUserRequests.setDefaultCreateAnonUserResponses(with: client) MockUserRequests.setDefaultIdentifyUserResponses(with: client, externalId: userA_EUID) - // The anonymous user needs its onesignal_id first, or the Identify User cannot address it. + // The Identify User addresses the anonymous user by onesignal_id, so let its Create User finish. OneSignalUserManagerImpl.sharedInstance.start() OneSignalCoreMocks.waitUntil("The anonymous user was not created") { OneSignalUserManagerImpl.sharedInstance.user.identityModel.onesignalId == anonUserOSID && self.client.isIdle @@ -137,24 +131,20 @@ final class UserStateReportingTests: XCTestCase { OneSignalCoreMocks.waitUntil("The Identify User was not started") { self.client.startedRequestCount(ofType: OSRequestIdentifyUser.self) == 1 } - // Makes B current while A's Identify User is still in flight; B's Create User queues behind it. + // B becomes current while A's Identify User is in flight. OneSignalUserManagerImpl.sharedInstance.login(externalId: userB_EUID, token: nil) client.releaseHeldResponses() waitForTheLoginToSettle() - // Hydrated, so anything queued for A has its onesignal_id. + // Still hydrated, for anything queued behind it. XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance.identityModelRepo.get(externalId: userA_EUID)?.onesignalId, anonUserOSID) XCTAssertFalse(observer.states.contains { $0.externalId == userA_EUID }, "the app must not hear about A: \(observer.states)") XCTAssertEqual(observer.states.last?.externalId, userB_EUID) assertPersistedSnapshotNames(externalId: userB_EUID, onesignalId: userB_OSID) } - /** - The 3.x upgrade path, again with no Identity Verification. The fetch identifies an anonymous user, - and a `login` that lands before its response makes an identified user current. The fetch still - hydrates the anonymous model, which the Identify User queued behind it needs, but the app must not - hear an anonymous user it has already logged in over. - */ + /// The 3.x upgrade path. A `login` before the legacy player's fetch returns makes an identified user + /// current; the response still hydrates the anonymous model, but the app must not hear that user. func testAFetchIdentityBySubscriptionThatCompletesAfterALoginDoesNotReportTheAnonymousUser() { OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: false) let legacyPlayerId = "legacy_player_id" @@ -174,19 +164,19 @@ final class UserStateReportingTests: XCTestCase { ) client.holdResponses = true - // Migrates the legacy player into an anonymous user whose identity the held fetch supplies. + // Migrates the legacy player; its identity fetch is held. OneSignalUserManagerImpl.sharedInstance.start() let anonymousModel = OneSignalUserManagerImpl.sharedInstance.user.identityModel XCTAssertNil(anonymousModel.onesignalId) OneSignalCoreMocks.waitUntil("The Fetch Identity By Subscription was not started") { self.client.startedRequestCount(ofType: OSRequestFetchIdentityBySubscription.self) == 1 } - // Makes A current while the anonymous user's fetch is still in flight; A's Identify User queues behind it. + // A becomes current while the fetch is in flight. OneSignalUserManagerImpl.sharedInstance.login(externalId: userA_EUID, token: nil) client.releaseHeldResponses() waitForTheLoginToSettle() - // Hydrated, so the Identify User behind the fetch could address the anonymous user. + // Still hydrated, for the Identify User behind it. XCTAssertEqual(anonymousModel.onesignalId, legacyOnesignalId) XCTAssertFalse(observer.states.contains { $0.externalId == nil }, "the app must not hear the anonymous user: \(observer.states)") XCTAssertEqual(observer.states.last?.externalId, userA_EUID) From de1bb50097afd8a9b0fa55579445a9f3731b5e16 Mon Sep 17 00:00:00 2001 From: Nan Date: Mon, 21 Sep 2026 13:07:20 -0700 Subject: [PATCH 12/13] chore: [SDK-5137] keep onesignal_id across the clear before a Fetch User hydrates `clearData` kept only `external_id`, so for the moment between the clear and the hydrate a live user's model had no `onesignal_id`. `dropIdentifyUsersThatCanNeverPrepare` runs on every send and does not count a Fetch User as a supplier, so a `login` landing in that moment, with a send pass behind it, dropped the Identify User and persisted the drop. Keep `onesignal_id` too: the fetch is addressed by one of the two ids, so neither can change. The same doc comment claimed the next `login` corrects a fetch response that omits `external_id`. It does not; a same-user login returns early while the local alias still matches, so only a login as someone else replaces it. The comment and the test docs now say that. The replace-on-hydrate alternative raised in review is deferred to its own ticket. The clear test's `onesignal_id` assertion flips from nil to kept, which the previous commit fails by construction. --- .../OneSignalUser/Source/OSIdentityModel.swift | 15 ++++++--------- .../Source/Requests/OSUserRequest.swift | 2 +- .../OneSignalUserTests/OSIdentityModelTests.swift | 9 ++++----- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift index 08d35e3ef..06a362887 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift @@ -133,18 +133,15 @@ class OSIdentityModel: OSModel { } /** - Called to clear the model's data in preparation for hydration via a fetch user call. - - `external_id` stays. The fetch that follows is by `onesignal_id`, so it cannot change who the user - is, and its response overwrites the alias anyway. Blanking it would let work built in the gap before - that response, on another queue, read this user as anonymous: a Delta stamped with no owner is - dropped under Identity Verification before it is ever persisted. A response that omits `external_id` - no longer reads as anonymous either; only a server-side unlink produces one, and the next `login` - corrects it. + Keeps `onesignal_id` and `external_id` and drops every other alias, ahead of the Fetch User response + that fills the model back in. The fetch is addressed by one of those two, so neither can change, and + work built on another queue before the response lands has to keep reading this user as created and + identified. A response that omits `external_id` therefore leaves it in place; a same-user `login` is + a no-op, so only a login as someone else replaces it. */ func clearData() { lock.withLock { - self.aliases = self.aliases.filter { $0.key == OS_EXTERNAL_ID } + self.aliases = self.aliases.filter { $0.key == OS_ONESIGNAL_ID || $0.key == OS_EXTERNAL_ID } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index 327e7a31a..ac02149fe 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -56,7 +56,7 @@ extension OSUserRequest { judge it by that rather than by its `identityModel`. The live model is not the record of who the work was for. Its aliases are cleared and hydrated again - around every fetch (only `external_id` survives the clear, see `OSIdentityModel.clearData`), and the + around every fetch (only the two ids survive the clear, see `OSIdentityModel.clearData`), and the owner has to be what it was when the work was built, not what the model reads later. The stamp also matches how `OSDelta` carries `externalId`, which keeps a Delta and the Request built from it judged the same way. diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift index 0dc4f259c..01dbf284d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OSIdentityModelTests.swift @@ -61,9 +61,8 @@ final class OSIdentityModelTests: XCTestCase { // MARK: - clearData() - /// The fetch that follows a clear is by `onesignal_id`, so it cannot change who the user is, and work - /// built before its response must not read this user as anonymous. - func testClearDataKeepsTheExternalIdAndDropsEveryOtherAlias() { + /// Work built before the fetch response lands must still read this user as created and identified. + func testClearDataKeepsBothIdsAndDropsEveryOtherAlias() { let model = OSIdentityModel( aliases: [OS_ONESIGNAL_ID: userA_OSID, OS_EXTERNAL_ID: userA_EUID, "stale_label": "stale_value"], changeNotifier: OSEventProducer() @@ -71,13 +70,13 @@ final class OSIdentityModelTests: XCTestCase { model.clearData() + XCTAssertEqual(model.onesignalId, userA_OSID) XCTAssertEqual(model.externalId, userA_EUID) - XCTAssertNil(model.onesignalId) XCTAssertNil(model.aliases["stale_label"]) } /// The fetch response merges into what the clear kept, so one without `external_id` leaves the user - /// identified until the next `login`. + /// identified. func testHydrateAfterClearDataMergesIntoTheKeptExternalId() { let model = OSIdentityModel( aliases: [OS_ONESIGNAL_ID: userA_OSID, OS_EXTERNAL_ID: userA_EUID], From ca777c7347f9ce6013a841932fea93dcb8ac7a47 Mon Sep 17 00:00:00 2001 From: Nan Date: Mon, 21 Sep 2026 14:13:10 -0700 Subject: [PATCH 13/13] chore: [SDK-5137] move the restored-login tests into their own class UserExecutorTests is 348 body lines on this branch and 331 on 5.8-main, each under SwiftLint's 350-line type_body_length error, but the two additions meet in the merge and the merged class is 407 lines, which fails the Swift Lint job. Move the three restored-login tests and their three helpers into UserExecutorRestoredLoginTests in the same file, next to the archive tests that already live there, so nothing changes in the project file. The branch class drops to 272 lines and the merged one to 331. --- .../Executors/UserExecutorTests.swift | 141 ++++++++++-------- 1 file changed, 78 insertions(+), 63 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift index 21e146a70..62e667cf2 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift @@ -439,6 +439,84 @@ final class UserExecutorTests: XCTestCase { XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) } + /// `login` promotes while the requirement is still unknown, so turning out to require auth must not + /// strand that login: it becomes the Create User it would have been. + func testInSessionIdentifyUserBecomesACreateUserWhenIdentityVerificationIsRequired() { + /* Setup */ + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) + let mocks = Mocks() + MockUserRequests.setDefaultIdentifyUserResponses(with: mocks.client, externalId: userA_EUID, conflicted: false) + MockUserRequests.setDefaultCreateUserResponses(with: mocks.client, externalId: userA_EUID) + + let anonIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()) + let user = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: nil) + user.identityModel.jwtBearerToken = "token-a" + + /* When */ + mocks.userExecutor.identifyUser(externalId: userA_EUID, identityModelToIdentify: anonIdentityModel, identityModelToUpdate: user.identityModel) + OneSignalCoreMocks.waitUntil("In-session Identify was not reshaped into a Create User") { + mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self) + } + + /* Then */ + XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self)) + } + + /// A promotion whose user a later `login` has already replaced has no login left to carry over. + func testInSessionIdentifyUserForAReplacedUserIsDroppedWhenIdentityVerificationIsRequired() { + /* Setup */ + OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) + let mocks = Mocks() + MockUserRequests.setDefaultIdentifyUserResponses(with: mocks.client, externalId: userA_EUID, conflicted: false) + + let anonIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()) + let replacedIdentityModel = OSIdentityModel(aliases: [OS_EXTERNAL_ID: userA_EUID], changeNotifier: OSEventProducer()) + _ = OneSignalUserMocks.setUserManagerInternalUser(externalId: userB_EUID, onesignalId: nil) + + /* When */ + mocks.userExecutor.identifyUser(externalId: userA_EUID, identityModelToIdentify: anonIdentityModel, identityModelToUpdate: replacedIdentityModel) + allowAsyncWorkToRun() + + /* Then */ + XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) + XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self)) + } + + private func cacheUserRequests(_ requests: [OSUserRequest]) { + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: requests) + } + + private func makeIdentifyUserRequest() -> OSRequestIdentifyUser { + return OSRequestIdentifyUser( + aliasLabel: OS_EXTERNAL_ID, + aliasId: userA_EUID, + identityModelToIdentify: OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()), + identityModelToUpdate: OSIdentityModel(aliases: [OS_EXTERNAL_ID: userA_EUID], changeNotifier: OSEventProducer()) + ) + } + + private func makeAnonymousCreateUserRequest() -> OSRequestCreateUser { + let pushModel = OSSubscriptionModel(type: .push, address: nil, subscriptionId: nil, reachable: false, isDisabled: false, changeNotifier: OSEventProducer()) + return OSRequestCreateUser( + identityModel: OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()), + propertiesModel: OSPropertiesModel(changeNotifier: OSEventProducer()), + pushSubscriptionModel: pushModel, + originalPushToken: nil + ) + } +} + +/// Logins restored from the archive of a launch that ended before its requests went out. Split from +/// `UserExecutorTests` at SwiftLint's type body limit. +final class UserExecutorRestoredLoginTests: XCTestCase { + + override func setUpWithError() throws { + OneSignalCoreMocks.clearUserDefaults() + OneSignalUserMocks.reset() + OneSignalIdentifiers.currentAppId = "test-app-id" + } + /// The archive an offline first launch with a `login` leaves behind: the anonymous Create User has not /// been sent, so its user has no `onesignal_id` yet. The Identify User behind it has to wait for that /// response rather than be dropped at start. @@ -517,73 +595,10 @@ final class UserExecutorTests: XCTestCase { XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self, expectedCount: 1)) } - /// `login` promotes while the requirement is still unknown, so turning out to require auth must not - /// strand that login: it becomes the Create User it would have been. - func testInSessionIdentifyUserBecomesACreateUserWhenIdentityVerificationIsRequired() { - /* Setup */ - OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) - let mocks = Mocks() - MockUserRequests.setDefaultIdentifyUserResponses(with: mocks.client, externalId: userA_EUID, conflicted: false) - MockUserRequests.setDefaultCreateUserResponses(with: mocks.client, externalId: userA_EUID) - - let anonIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()) - let user = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: nil) - user.identityModel.jwtBearerToken = "token-a" - - /* When */ - mocks.userExecutor.identifyUser(externalId: userA_EUID, identityModelToIdentify: anonIdentityModel, identityModelToUpdate: user.identityModel) - OneSignalCoreMocks.waitUntil("In-session Identify was not reshaped into a Create User") { - mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self) - } - - /* Then */ - XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) - XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self)) - } - - /// A promotion whose user a later `login` has already replaced has no login left to carry over. - func testInSessionIdentifyUserForAReplacedUserIsDroppedWhenIdentityVerificationIsRequired() { - /* Setup */ - OSCoreMocks.hydrateSharedJwtConfig(requiresUserAuth: true) - let mocks = Mocks() - MockUserRequests.setDefaultIdentifyUserResponses(with: mocks.client, externalId: userA_EUID, conflicted: false) - - let anonIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()) - let replacedIdentityModel = OSIdentityModel(aliases: [OS_EXTERNAL_ID: userA_EUID], changeNotifier: OSEventProducer()) - _ = OneSignalUserMocks.setUserManagerInternalUser(externalId: userB_EUID, onesignalId: nil) - - /* When */ - mocks.userExecutor.identifyUser(externalId: userA_EUID, identityModelToIdentify: anonIdentityModel, identityModelToUpdate: replacedIdentityModel) - allowAsyncWorkToRun() - - /* Then */ - XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestIdentifyUser.self)) - XCTAssertFalse(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self)) - } - private func cacheUserRequests(_ requests: [OSUserRequest]) { OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: requests) } - private func makeIdentifyUserRequest() -> OSRequestIdentifyUser { - return OSRequestIdentifyUser( - aliasLabel: OS_EXTERNAL_ID, - aliasId: userA_EUID, - identityModelToIdentify: OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()), - identityModelToUpdate: OSIdentityModel(aliases: [OS_EXTERNAL_ID: userA_EUID], changeNotifier: OSEventProducer()) - ) - } - - private func makeAnonymousCreateUserRequest() -> OSRequestCreateUser { - let pushModel = OSSubscriptionModel(type: .push, address: nil, subscriptionId: nil, reachable: false, isDisabled: false, changeNotifier: OSEventProducer()) - return OSRequestCreateUser( - identityModel: OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer()), - propertiesModel: OSPropertiesModel(changeNotifier: OSEventProducer()), - pushSubscriptionModel: pushModel, - originalPushToken: nil - ) - } - private func makeIdentifyUserRequest( identifying identityModelToIdentify: OSIdentityModel, updating identityModelToUpdate: OSIdentityModel