From 714d53d0858b17337d4a854eb274f4719eaff17b Mon Sep 17 00:00:00 2001 From: Nan Date: Tue, 19 May 2026 09:20:45 -0700 Subject: [PATCH 1/3] fix(jwt): make OSIdentityModel.jwtBearerToken thread-safe fix unsynchronized reads of OSIdentityModel state (jwtBearerToken in particular) --- .../Source/Modeling/OSIdentityModel.swift | 47 +++++++++++++------ .../Source/OneSignalUserManagerImpl.swift | 4 +- .../Source/Requests/OSUserRequest.swift | 10 ++-- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift index dcbe778d7..b59334d0b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift @@ -38,23 +38,42 @@ class OSIdentityModel: OSModel { return internalGetAlias(OS_EXTERNAL_ID) } - // All access to aliases should go through helper methods with locking + // All access to aliases and jwtBearerToken must go through the lock var aliases: [String: String] = [:] - private let aliasesLock = NSRecursiveLock() + private let lock = NSRecursiveLock() // MARK: - JWT + private var _jwtBearerToken: String? public var jwtBearerToken: String? { - didSet { - guard jwtBearerToken != oldValue else { - return + get { + lock.withLock { _jwtBearerToken } + } + set { + // Lock only the storage write. The change notifier fires synchronously + // to listeners that may take other locks; firing under our lock would + // risk deadlock (NSRecursiveLock only saves same-thread re-entry). + let changed: Bool = lock.withLock { + guard newValue != _jwtBearerToken else { return false } + _jwtBearerToken = newValue + return true + } + if changed { + self.set(property: OS_JWT_BEARER_TOKEN, newValue: newValue) } - self.set(property: OS_JWT_BEARER_TOKEN, newValue: jwtBearerToken) } } - func isJwtValid() -> Bool { - return jwtBearerToken != nil && jwtBearerToken != "" && jwtBearerToken != OS_JWT_TOKEN_INVALID + /// Returns the bearer token if it is non-nil, non-empty, and not the + /// `OS_JWT_TOKEN_INVALID` sentinel — otherwise nil. Snapshots once so the + /// caller cannot split a read-then-check across two reads of a property + /// that other threads can mutate. + func getValidJwt() -> String? { + let token = jwtBearerToken + guard let token = token, !token.isEmpty, token != OS_JWT_TOKEN_INVALID else { + return nil + } + return token } // MARK: - Initialization @@ -66,10 +85,10 @@ class OSIdentityModel: OSModel { } override func encode(with coder: NSCoder) { - aliasesLock.withLock { + lock.withLock { super.encode(with: coder) coder.encode(aliases, forKey: "aliases") - coder.encode(jwtBearerToken, forKey: OS_JWT_BEARER_TOKEN) + coder.encode(_jwtBearerToken, forKey: OS_JWT_BEARER_TOKEN) } } @@ -79,20 +98,20 @@ class OSIdentityModel: OSModel { // log error return nil } - self.jwtBearerToken = coder.decodeObject(forKey: OS_JWT_BEARER_TOKEN) as? String + self._jwtBearerToken = coder.decodeObject(forKey: OS_JWT_BEARER_TOKEN) as? String self.aliases = aliases } /** Threadsafe getter for an alias */ private func internalGetAlias(_ label: String) -> String? { - aliasesLock.withLock { + lock.withLock { return self.aliases[label] } } /** Threadsafe setter or removal for aliases */ private func internalAddAliases(_ aliases: [String: String]) { - aliasesLock.withLock { + lock.withLock { for (label, id) in aliases { // Remove the alias if the ID field is "" self.aliases[label] = id.isEmpty ? nil : id @@ -105,7 +124,7 @@ class OSIdentityModel: OSModel { Called to clear the model's data in preparation for hydration via a fetch user call. */ func clearData() { - aliasesLock.withLock { + lock.withLock { self.aliases = [:] } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index 2db7d5381..ae0eb8afe 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -412,9 +412,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { // JWT is required - if _user.identityModel.isJwtValid(), - let token = _user.identityModel.jwtBearerToken - { + if let token = _user.identityModel.getValidJwt() { fullHeader["Authorization"] = "Bearer \(token)" return fullHeader } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index 52bebf57e..d0a696133 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -70,12 +70,12 @@ internal extension OneSignalRequest { | --------------- | -------------- | ------- | ------- | */ func addJWTHeaderIsValid(identityModel: OSIdentityModel) -> Bool { - let tokenIsValid = identityModel.isJwtValid() + // Snapshot once via getValidJwt() to avoid split read-then-check races + // between concurrent writers (login/setUserJwtToken/invalidate). + let validToken = identityModel.getValidJwt() let required = OneSignalUserManagerImpl.sharedInstance.jwtConfig.isRequired - let canBeSent = (required == false) || (required == true && tokenIsValid) - if canBeSent && tokenIsValid, - let token = identityModel.jwtBearerToken - { + let canBeSent = (required == false) || (required == true && validToken != nil) + if canBeSent, let token = validToken { // Add the JWT token if it is valid, regardless of requirements var additionalHeaders = self.additionalHeaders ?? [String: String]() additionalHeaders["Authorization"] = "Bearer \(token)" From 51cabc67b92afad856293be8eb58f5e5a29faf30 Mon Sep 17 00:00:00 2001 From: Nan Date: Tue, 19 May 2026 09:21:34 -0700 Subject: [PATCH 2/3] fix(jwt): remove notifier-under-lock and TOCTOU in JWT invalidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-on JWT concurrency issues exposed while reviewing the prior fix. 1. OSIdentityModelRepo.updateJwtToken fired the model's change notifier synchronously (→ onModelUpdated → onJwtTokenChanged → executor listeners) while still holding the repo's NSLock. Today nothing re-enters the repo lock so it doesn't deadlock by luck, but it's a trap for any future listener. The fix collects matching models under the lock and mutates them outside, so the notifier fires lock-free. 2. invalidateJwtForExternalId had a TOCTOU between its "is it already invalid?" read and the "set to invalid" write. A concurrent valid-token write landing between them would be overwritten with INVALID and trigger a needless re-auth. The transition is now an atomic compare-and-set on the model (invalidateJwtBearerToken); only the thread that wins the transition fires fireJwtExpired. Co-Authored-By: Claude Opus 4.7 --- .../Source/Modeling/OSIdentityModel.swift | 19 ++++++++++++++++++ .../Source/OSIdentityModelRepo.swift | 20 ++++++++++--------- .../Source/OneSignalUserManagerImpl.swift | 13 ++++++------ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift index b59334d0b..26e25c567 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift @@ -76,6 +76,25 @@ class OSIdentityModel: OSModel { return token } + /** + Atomically transition the JWT token to `OS_JWT_TOKEN_INVALID`. Returns + `true` if the transition occurred, `false` if the token was already + invalid. Used by `invalidateJwtForExternalId` so only the thread that + actually invalidated fires `fireJwtExpired`. + */ + @discardableResult + func invalidateJwtBearerToken() -> Bool { + let changed: Bool = lock.withLock { + guard _jwtBearerToken != OS_JWT_TOKEN_INVALID else { return false } + _jwtBearerToken = OS_JWT_TOKEN_INVALID + return true + } + if changed { + self.set(property: OS_JWT_BEARER_TOKEN, newValue: OS_JWT_TOKEN_INVALID) + } + return changed + } + // MARK: - Initialization // Initialize with aliases, if any diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModelRepo.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModelRepo.swift index ef82e264e..37e7e007e 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModelRepo.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModelRepo.swift @@ -73,17 +73,19 @@ class OSIdentityModelRepo { This can be optimized in the future to re-use an Identity Model if multiple logins are made for the same user. */ func updateJwtToken(externalId: String, token: String) { - var found = false - lock.withLock { - for model in models.values { - if model.externalId == externalId { - model.jwtBearerToken = token - found = true - } - } + // Snapshot matching models under the repo lock, then mutate outside. + // Writing the token fires the model's change notifier synchronously + // (→ onModelUpdated → onJwtTokenChanged); doing that while holding the + // repo lock leaves a trap for future listeners to deadlock on. + let matchingModels: [OSIdentityModel] = lock.withLock { + models.values.filter { $0.externalId == externalId } } - if !found { + guard !matchingModels.isEmpty else { OneSignalLog.onesignalLog(ONE_S_LOG_LEVEL.LL_ERROR, message: "Update User JWT called for external ID \(externalId) that does not exist") + return + } + for model in matchingModels { + model.jwtBearerToken = token } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index ae0eb8afe..5854d85ef 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -714,14 +714,13 @@ extension OneSignalUserManagerImpl { return } - // Return, if the token has already been invalidated - guard identityModel.jwtBearerToken != OS_JWT_TOKEN_INVALID else { - return + // Atomic compare-and-set on the model. Only the thread that actually + // transitioned the token to INVALID fires the expired event — avoids + // a needless re-auth round trip if a concurrent valid-token write + // landed between a TOCTOU read/write pair. + if identityModel.invalidateJwtBearerToken() { + fireJwtExpired(externalId: externalId) } - - identityModel.jwtBearerToken = OS_JWT_TOKEN_INVALID - - fireJwtExpired(externalId: externalId) } private func fireJwtExpired(externalId: String) { From ea743809d9fd760876e802a40f84d406796d89a5 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 28 May 2026 10:31:50 -0700 Subject: [PATCH 3/3] cleanup renaming a variable and cleaning up comments --- .../Source/Modeling/OSIdentityModel.swift | 32 ++++++++----------- .../Source/OneSignalUserManagerImpl.swift | 4 --- .../Source/Requests/OSUserRequest.swift | 2 -- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift index 26e25c567..2c5d158e0 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift @@ -44,18 +44,17 @@ class OSIdentityModel: OSModel { // MARK: - JWT - private var _jwtBearerToken: String? + private var jwtBearerTokenLocked: String? // only read/write under self.lock public var jwtBearerToken: String? { get { - lock.withLock { _jwtBearerToken } + lock.withLock { jwtBearerTokenLocked } } set { // Lock only the storage write. The change notifier fires synchronously - // to listeners that may take other locks; firing under our lock would - // risk deadlock (NSRecursiveLock only saves same-thread re-entry). - let changed: Bool = lock.withLock { - guard newValue != _jwtBearerToken else { return false } - _jwtBearerToken = newValue + // to listeners that may take other locks + let changed = lock.withLock { + guard newValue != jwtBearerTokenLocked else { return false } + jwtBearerTokenLocked = newValue return true } if changed { @@ -64,10 +63,7 @@ class OSIdentityModel: OSModel { } } - /// Returns the bearer token if it is non-nil, non-empty, and not the - /// `OS_JWT_TOKEN_INVALID` sentinel — otherwise nil. Snapshots once so the - /// caller cannot split a read-then-check across two reads of a property - /// that other threads can mutate. + /// Returns the bearer token if it is valid, otherwise nil, snapshots once func getValidJwt() -> String? { let token = jwtBearerToken guard let token = token, !token.isEmpty, token != OS_JWT_TOKEN_INVALID else { @@ -78,15 +74,13 @@ class OSIdentityModel: OSModel { /** Atomically transition the JWT token to `OS_JWT_TOKEN_INVALID`. Returns - `true` if the transition occurred, `false` if the token was already - invalid. Used by `invalidateJwtForExternalId` so only the thread that - actually invalidated fires `fireJwtExpired`. + `true` if the transition occurred, `false` if the token was already invalid. */ @discardableResult func invalidateJwtBearerToken() -> Bool { - let changed: Bool = lock.withLock { - guard _jwtBearerToken != OS_JWT_TOKEN_INVALID else { return false } - _jwtBearerToken = OS_JWT_TOKEN_INVALID + let changed = lock.withLock { + guard jwtBearerTokenLocked != OS_JWT_TOKEN_INVALID else { return false } + jwtBearerTokenLocked = OS_JWT_TOKEN_INVALID return true } if changed { @@ -107,7 +101,7 @@ class OSIdentityModel: OSModel { lock.withLock { super.encode(with: coder) coder.encode(aliases, forKey: "aliases") - coder.encode(_jwtBearerToken, forKey: OS_JWT_BEARER_TOKEN) + coder.encode(jwtBearerTokenLocked, forKey: OS_JWT_BEARER_TOKEN) } } @@ -117,7 +111,7 @@ class OSIdentityModel: OSModel { // log error return nil } - self._jwtBearerToken = coder.decodeObject(forKey: OS_JWT_BEARER_TOKEN) as? String + self.jwtBearerTokenLocked = coder.decodeObject(forKey: OS_JWT_BEARER_TOKEN) as? String self.aliases = aliases } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index 5854d85ef..5fe73b4e8 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -714,10 +714,6 @@ extension OneSignalUserManagerImpl { return } - // Atomic compare-and-set on the model. Only the thread that actually - // transitioned the token to INVALID fires the expired event — avoids - // a needless re-auth round trip if a concurrent valid-token write - // landed between a TOCTOU read/write pair. if identityModel.invalidateJwtBearerToken() { fireJwtExpired(externalId: externalId) } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift index d0a696133..fa435d421 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSUserRequest.swift @@ -70,8 +70,6 @@ internal extension OneSignalRequest { | --------------- | -------------- | ------- | ------- | */ func addJWTHeaderIsValid(identityModel: OSIdentityModel) -> Bool { - // Snapshot once via getValidJwt() to avoid split read-then-check races - // between concurrent writers (login/setUserJwtToken/invalidate). let validToken = identityModel.getValidJwt() let required = OneSignalUserManagerImpl.sharedInstance.jwtConfig.isRequired let canBeSent = (required == false) || (required == true && validToken != nil)