Skip to content

Commit bb8284c

Browse files
nan-liclaude
andauthored
fix: unsynchronized reads of OSIdentityModel state (jwt) (#1665)
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 630b020 commit bb8284c

11 files changed

Lines changed: 219 additions & 54 deletions

File tree

examples/demo/App/Models/AppModels.swift

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ enum AddItemType {
8080
case tag
8181
case trigger
8282
case externalUserId
83+
case updateJwt
8384

8485
var title: String {
8586
switch self {
@@ -89,20 +90,31 @@ enum AddItemType {
8990
case .tag: return "Add Tag"
9091
case .trigger: return "Add Trigger"
9192
case .externalUserId: return "Login User"
93+
case .updateJwt: return "Update JWT"
9294
}
9395
}
9496

9597
var requiresKeyValue: Bool {
9698
switch self {
97-
case .alias, .tag, .trigger: return true
98-
case .email, .sms, .externalUserId: return false
99+
case .alias, .tag, .trigger, .externalUserId, .updateJwt: return true
100+
case .email, .sms: return false
101+
}
102+
}
103+
104+
/// When true the second field may be left blank (confirm stays enabled).
105+
/// Used by Login, where the JWT token is optional.
106+
var optionalValue: Bool {
107+
switch self {
108+
case .externalUserId: return true
109+
default: return false
99110
}
100111
}
101112

102113
var keyPlaceholder: String {
103114
switch self {
104115
case .alias: return "Label"
105116
case .tag, .trigger: return "Key"
117+
case .externalUserId, .updateJwt: return "External User Id"
106118
default: return "Key"
107119
}
108120
}
@@ -113,7 +125,8 @@ enum AddItemType {
113125
case .email: return "Email Address"
114126
case .sms: return "Phone Number"
115127
case .tag, .trigger: return "Value"
116-
case .externalUserId: return "External User Id"
128+
case .externalUserId: return "JWT Token (optional)"
129+
case .updateJwt: return "JWT Token"
117130
}
118131
}
119132

@@ -128,6 +141,7 @@ enum AddItemType {
128141
var confirmLabel: String {
129142
switch self {
130143
case .externalUserId: return "Login"
144+
case .updateJwt: return "Update"
131145
default: return "Add"
132146
}
133147
}
@@ -141,6 +155,7 @@ enum AddItemType {
141155
case .tag: return "tag"
142156
case .trigger: return "trigger"
143157
case .externalUserId: return "login_user_id"
158+
case .updateJwt: return "update_jwt"
144159
}
145160
}
146161

@@ -152,6 +167,8 @@ enum AddItemType {
152167
case .alias: return "alias_label_input"
153168
case .tag: return "tag_key_input"
154169
case .trigger: return "trigger_key_input"
170+
case .externalUserId: return "login_user_id_input"
171+
case .updateJwt: return "update_jwt_external_id_input"
155172
default: return "\(accessibilityKey)_key_input"
156173
}
157174
}
@@ -165,6 +182,8 @@ enum AddItemType {
165182
case .alias: return "alias_id_input"
166183
case .tag: return "tag_value_input"
167184
case .trigger: return "trigger_value_input"
185+
case .externalUserId: return "login_user_jwt_input"
186+
case .updateJwt: return "update_jwt_token_input"
168187
default: return "\(accessibilityKey)_input"
169188
}
170189
}

examples/demo/App/Services/OneSignalService.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,13 @@ final class OneSignalService {
9696

9797
// MARK: - User
9898

99-
func login(externalId: String) {
99+
func login(externalId: String, token: String? = nil) {
100100
prefs.setExternalUserId(externalId)
101-
OneSignal.login(externalId)
101+
OneSignal.login(externalId: externalId, token: token)
102+
}
103+
104+
func updateUserJwt(externalId: String, token: String) {
105+
OneSignal.updateUserJwt(externalId: externalId, token: token)
102106
}
103107

104108
func logout() {

examples/demo/App/ViewModels/OneSignalViewModel.swift

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ final class OneSignalViewModel: ObservableObject {
6767

6868
// MARK: - UI State
6969

70-
@Published var isLoading: Bool = false
71-
7270
@Published var activeTooltip: TooltipData?
7371

7472
// MARK: - Computed
@@ -129,7 +127,6 @@ final class OneSignalViewModel: ObservableObject {
129127
guard let onesignalId = service.onesignalId else { return }
130128
requestSequence &+= 1
131129
let captured = requestSequence
132-
isLoading = true
133130

134131
let userData = await UserFetchService.shared.fetchUser(appId: appId, onesignalId: onesignalId)
135132

@@ -145,7 +142,6 @@ final class OneSignalViewModel: ObservableObject {
145142
externalUserId = extId
146143
}
147144
}
148-
isLoading = false
149145
}
150146

151147
// MARK: - Consent
@@ -166,15 +162,22 @@ final class OneSignalViewModel: ObservableObject {
166162

167163
// MARK: - User
168164

169-
func login(externalId: String) {
165+
func login(externalId: String, token: String? = nil) {
170166
let trimmed = externalId.trimmingCharacters(in: .whitespacesAndNewlines)
171167
guard !trimmed.isEmpty else { return }
172-
isLoading = true
173-
service.login(externalId: trimmed)
168+
let trimmedToken = token?.trimmingCharacters(in: .whitespacesAndNewlines)
169+
service.login(externalId: trimmed, token: (trimmedToken?.isEmpty ?? true) ? nil : trimmedToken)
174170
externalUserId = trimmed
175171
clearUserData()
176172
}
177173

174+
func updateUserJwt(externalId: String, token: String) {
175+
let trimmedId = externalId.trimmingCharacters(in: .whitespacesAndNewlines)
176+
let trimmedToken = token.trimmingCharacters(in: .whitespacesAndNewlines)
177+
guard !trimmedId.isEmpty, !trimmedToken.isEmpty else { return }
178+
service.updateUserJwt(externalId: trimmedId, token: trimmedToken)
179+
}
180+
178181
func logout() {
179182
service.logout()
180183
externalUserId = nil

examples/demo/App/Views/Components/AddItemDialog.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,9 @@ struct AddItemDialog: View {
8080

8181
private var isValid: Bool {
8282
if itemType.requiresKeyValue {
83-
return !keyText.trimmingCharacters(in: .whitespaces).isEmpty &&
84-
!valueText.trimmingCharacters(in: .whitespaces).isEmpty
83+
let keyOK = !keyText.trimmingCharacters(in: .whitespaces).isEmpty
84+
if itemType.optionalValue { return keyOK }
85+
return keyOK && !valueText.trimmingCharacters(in: .whitespaces).isEmpty
8586
}
8687
return !valueText.trimmingCharacters(in: .whitespaces).isEmpty
8788
}

examples/demo/App/Views/Sections/UserSection.swift

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import SwiftUI
3131
struct UserSection: View {
3232
@EnvironmentObject var viewModel: OneSignalViewModel
3333
@State private var loginOpen = false
34+
@State private var updateJwtOpen = false
3435

3536
var body: some View {
3637
SectionCard(title: "USER", sectionKey: "user") {
@@ -55,6 +56,14 @@ struct UserSection: View {
5556
loginOpen = true
5657
}
5758

59+
ActionButton(
60+
"UPDATE JWT",
61+
style: .outline,
62+
accessibilityID: "update_jwt_button"
63+
) {
64+
updateJwtOpen = true
65+
}
66+
5867
if viewModel.isLoggedIn {
5968
ActionButton(
6069
"LOGOUT USER",
@@ -68,12 +77,22 @@ struct UserSection: View {
6877
.osCenteredDialog(isPresented: $loginOpen) {
6978
AddItemDialog(
7079
itemType: .externalUserId,
71-
onAdd: { _, value in
72-
viewModel.login(externalId: value)
80+
onAdd: { externalId, token in
81+
viewModel.login(externalId: externalId, token: token.isEmpty ? nil : token)
7382
loginOpen = false
7483
},
7584
onCancel: { loginOpen = false }
7685
)
7786
}
87+
.osCenteredDialog(isPresented: $updateJwtOpen) {
88+
AddItemDialog(
89+
itemType: .updateJwt,
90+
onAdd: { externalId, token in
91+
viewModel.updateUserJwt(externalId: externalId, token: token)
92+
updateJwtOpen = false
93+
},
94+
onCancel: { updateJwtOpen = false }
95+
)
96+
}
7897
}
7998
}

iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@
185185
3CC063E02B6D7F2A002BB07F /* OneSignalUserMocks.h in Headers */ = {isa = PBXBuildFile; fileRef = 3CC063DF2B6D7F2A002BB07F /* OneSignalUserMocks.h */; settings = {ATTRIBUTES = (Public, ); }; };
186186
3CC063E62B6D7F96002BB07F /* OneSignalUserMocks.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CC063E52B6D7F96002BB07F /* OneSignalUserMocks.swift */; };
187187
3CC063EE2B6D7FE8002BB07F /* OneSignalUserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CC063ED2B6D7FE8002BB07F /* OneSignalUserTests.swift */; };
188+
B91A66287DEA4026A4DC5952 /* OSIdentityModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6C1399651D1A401EB888DA77 /* OSIdentityModelTests.swift */; };
188189
3CC063EF2B6D7FE8002BB07F /* OneSignalUser.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DE69E19B282ED8060090BB3D /* OneSignalUser.framework */; };
189190
3CC890352C5BF9A7002CB4CC /* UserConcurrencyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CC890342C5BF9A7002CB4CC /* UserConcurrencyTests.swift */; };
190191
3CC9A6342AFA1FDE008F68FD /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = 3CC9A6332AFA1FDD008F68FD /* PrivacyInfo.xcprivacy */; };
@@ -1439,6 +1440,7 @@
14391440
3CC063E52B6D7F96002BB07F /* OneSignalUserMocks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OneSignalUserMocks.swift; sourceTree = "<group>"; };
14401441
3CC063EB2B6D7FE8002BB07F /* OneSignalUserTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = OneSignalUserTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
14411442
3CC063ED2B6D7FE8002BB07F /* OneSignalUserTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OneSignalUserTests.swift; sourceTree = "<group>"; };
1443+
6C1399651D1A401EB888DA77 /* OSIdentityModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIdentityModelTests.swift; sourceTree = "<group>"; };
14421444
3CC890342C5BF9A7002CB4CC /* UserConcurrencyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserConcurrencyTests.swift; sourceTree = "<group>"; };
14431445
3CC9A6332AFA1FDD008F68FD /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
14441446
3CC9A6352AFA26E7008F68FD /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
@@ -2422,6 +2424,7 @@
24222424
3CDE664A2BFC2A55006DA114 /* OneSignalUserTests-Bridging-Header.h */,
24232425
3CF11E3E2C6D61AC002856F5 /* Executors */,
24242426
3CC063ED2B6D7FE8002BB07F /* OneSignalUserTests.swift */,
2427+
6C1399651D1A401EB888DA77 /* OSIdentityModelTests.swift */,
24252428
3CC890342C5BF9A7002CB4CC /* UserConcurrencyTests.swift */,
24262429
3CB331672F281679000E1801 /* CustomEventsIntegrationTests.swift */,
24272430
3C67F7792BEB2B710085A0F0 /* SwitchUserIntegrationTests.swift */,
@@ -4539,6 +4542,7 @@
45394542
DE3568F22C8911EA00AF447C /* IdentityExecutorTests.swift in Sources */,
45404543
3C67F77A2BEB2B710085A0F0 /* SwitchUserIntegrationTests.swift in Sources */,
45414544
3CC063EE2B6D7FE8002BB07F /* OneSignalUserTests.swift in Sources */,
4545+
B91A66287DEA4026A4DC5952 /* OSIdentityModelTests.swift in Sources */,
45424546
3CC890352C5BF9A7002CB4CC /* UserConcurrencyTests.swift in Sources */,
45434547
DE3568F02C89067400AF447C /* SubscriptionsExecutorTests.swift in Sources */,
45444548
3CB3316A2F281692000E1801 /* OSCustomEventsExecutorTests.swift in Sources */,

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSIdentityModel.swift

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,23 +38,55 @@ class OSIdentityModel: OSModel {
3838
return internalGetAlias(OS_EXTERNAL_ID)
3939
}
4040

41-
// All access to aliases should go through helper methods with locking
41+
// All access to aliases and jwtBearerToken must go through the lock
4242
var aliases: [String: String] = [:]
43-
private let aliasesLock = NSRecursiveLock()
43+
private let lock = NSRecursiveLock()
4444

4545
// MARK: - JWT
4646

47+
private var jwtBearerTokenLocked: String? // only read/write under self.lock
4748
public var jwtBearerToken: String? {
48-
didSet {
49-
guard jwtBearerToken != oldValue else {
50-
return
49+
get {
50+
lock.withLock { jwtBearerTokenLocked }
51+
}
52+
set {
53+
// Lock only the storage write. The change notifier fires synchronously
54+
// to listeners that may take other locks
55+
let changed = lock.withLock {
56+
guard newValue != jwtBearerTokenLocked else { return false }
57+
jwtBearerTokenLocked = newValue
58+
return true
59+
}
60+
if changed {
61+
self.set(property: OS_JWT_BEARER_TOKEN, newValue: newValue)
5162
}
52-
self.set(property: OS_JWT_BEARER_TOKEN, newValue: jwtBearerToken)
5363
}
5464
}
5565

56-
func isJwtValid() -> Bool {
57-
return jwtBearerToken != nil && jwtBearerToken != "" && jwtBearerToken != OS_JWT_TOKEN_INVALID
66+
/// Returns the bearer token if it is valid, otherwise nil, snapshots once
67+
func getValidJwt() -> String? {
68+
let token = jwtBearerToken
69+
guard let token = token, !token.isEmpty, token != OS_JWT_TOKEN_INVALID else {
70+
return nil
71+
}
72+
return token
73+
}
74+
75+
/**
76+
Atomically transition the JWT token to `OS_JWT_TOKEN_INVALID`. Returns
77+
`true` if the transition occurred, `false` if the token was already invalid.
78+
*/
79+
@discardableResult
80+
func invalidateJwtBearerToken() -> Bool {
81+
let changed = lock.withLock {
82+
guard jwtBearerTokenLocked != OS_JWT_TOKEN_INVALID else { return false }
83+
jwtBearerTokenLocked = OS_JWT_TOKEN_INVALID
84+
return true
85+
}
86+
if changed {
87+
self.set(property: OS_JWT_BEARER_TOKEN, newValue: OS_JWT_TOKEN_INVALID)
88+
}
89+
return changed
5890
}
5991

6092
// MARK: - Initialization
@@ -66,10 +98,10 @@ class OSIdentityModel: OSModel {
6698
}
6799

68100
override func encode(with coder: NSCoder) {
69-
aliasesLock.withLock {
101+
lock.withLock {
70102
super.encode(with: coder)
71103
coder.encode(aliases, forKey: "aliases")
72-
coder.encode(jwtBearerToken, forKey: OS_JWT_BEARER_TOKEN)
104+
coder.encode(jwtBearerTokenLocked, forKey: OS_JWT_BEARER_TOKEN)
73105
}
74106
}
75107

@@ -79,20 +111,20 @@ class OSIdentityModel: OSModel {
79111
// log error
80112
return nil
81113
}
82-
self.jwtBearerToken = coder.decodeObject(forKey: OS_JWT_BEARER_TOKEN) as? String
114+
self.jwtBearerTokenLocked = coder.decodeObject(forKey: OS_JWT_BEARER_TOKEN) as? String
83115
self.aliases = aliases
84116
}
85117

86118
/** Threadsafe getter for an alias */
87119
private func internalGetAlias(_ label: String) -> String? {
88-
aliasesLock.withLock {
120+
lock.withLock {
89121
return self.aliases[label]
90122
}
91123
}
92124

93125
/** Threadsafe setter or removal for aliases */
94126
private func internalAddAliases(_ aliases: [String: String]) {
95-
aliasesLock.withLock {
127+
lock.withLock {
96128
for (label, id) in aliases {
97129
// Remove the alias if the ID field is ""
98130
self.aliases[label] = id.isEmpty ? nil : id
@@ -105,7 +137,7 @@ class OSIdentityModel: OSModel {
105137
Called to clear the model's data in preparation for hydration via a fetch user call.
106138
*/
107139
func clearData() {
108-
aliasesLock.withLock {
140+
lock.withLock {
109141
self.aliases = [:]
110142
}
111143
}

iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModelRepo.swift

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,19 @@ class OSIdentityModelRepo {
7373
This can be optimized in the future to re-use an Identity Model if multiple logins are made for the same user.
7474
*/
7575
func updateJwtToken(externalId: String, token: String) {
76-
var found = false
77-
lock.withLock {
78-
for model in models.values {
79-
if model.externalId == externalId {
80-
model.jwtBearerToken = token
81-
found = true
82-
}
83-
}
76+
// Snapshot matching models under the repo lock, then mutate outside.
77+
// Writing the token fires the model's change notifier synchronously
78+
// (→ onModelUpdated → onJwtTokenChanged); doing that while holding the
79+
// repo lock leaves a trap for future listeners to deadlock on.
80+
let matchingModels: [OSIdentityModel] = lock.withLock {
81+
models.values.filter { $0.externalId == externalId }
8482
}
85-
if !found {
83+
guard !matchingModels.isEmpty else {
8684
OneSignalLog.onesignalLog(ONE_S_LOG_LEVEL.LL_ERROR, message: "Update User JWT called for external ID \(externalId) that does not exist")
85+
return
86+
}
87+
for model in matchingModels {
88+
model.jwtBearerToken = token
8789
}
8890
}
8991
}

0 commit comments

Comments
 (0)