Skip to content

fix(jwt): race conditions and IAM reliability issues with identity verification enabled #2609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nan-li wants to merge 7 commits into
base: feat/identity_verification_5.8
Choose a base branch
from
Open

fix(jwt): race conditions and IAM reliability issues with identity verification enabled #2609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e38f225
Fix race condition: purge anonymous ops AFTER queue is loaded
nan-li Apr 9, 2026
4f086c6
Replay JWT invalidated event to late-registered listeners
nan-li Apr 9, 2026
098a4f7
Fix IAM fetch stuck after login when identity verification is enabled
nan-li Apr 9, 2026
c61f42e
Retry IAM fetch after JWT refresh on 401/403 response
nan-li Apr 9, 2026
60c8d9a
chore: fix for detekt, add doc, simplify return
nan-li Apr 9, 2026
cfc3e6e
Fix PR review issues: fallback 401 handling, volatile fields, TOCTOU …
nan-li Apr 10, 2026
569659c
Fix rate-limiter visibility and ConsistencyManager data race
nan-li Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions OneSignalSDK/detekt/detekt-baseline-core.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,6 @@
<ID>ForbiddenComment:HttpClient.kt$HttpClient$// TODO: SHOULD RETURN OK INSTEAD OF NOT_MODIFIED TO MAKE TRANSPARENT?</ID>
<ID>ForbiddenComment:IPreferencesService.kt$PreferenceOneSignalKeys$* (String) The serialized IAMs TODO: This isn't currently used, determine if actually needed for cold start IAM fetch delay</ID>
<ID>ForbiddenComment:IUserBackendService.kt$IUserBackendService$// TODO: Change to send only the push subscription, optimally</ID>
<ID>ForbiddenComment:LoginHelper.kt$LoginHelper$// TODO: Set JWT Token for all future requests.</ID>
<ID>ForbiddenComment:LogoutHelper.kt$LogoutHelper$// TODO: remove JWT Token for all future requests.</ID>
<ID>ForbiddenComment:OperationRepo.kt$OperationRepo$// TODO: Need to provide callback for app to reset JWT. For now, fail with no retry.</ID>
<ID>ForbiddenComment:ParamsBackendService.kt$ParamsBackendService$// TODO: New</ID>
<ID>ForbiddenComment:PermissionsActivity.kt$PermissionsActivity$// TODO after we remove IAM from being an activity window we may be able to remove this handler</ID>
<ID>ForbiddenComment:PermissionsActivity.kt$PermissionsActivity$// TODO improve this method</ID>
<ID>ForbiddenComment:PermissionsViewModel.kt$PermissionsViewModel.Companion$// TODO this will be removed once the handler is deleted</ID>
Expand Down Expand Up @@ -217,7 +213,7 @@
<ID>LongParameterList:IOutcomeEventsBackendService.kt$IOutcomeEventsBackendService$( appId: String, userId: String, subscriptionId: String, deviceType: String, direct: Boolean?, event: OutcomeEvent, )</ID>
<ID>LongParameterList:IParamsBackendService.kt$ParamsObject$( var googleProjectNumber: String? = null, var enterprise: Boolean? = null, var useIdentityVerification: Boolean? = null, var notificationChannels: JSONArray? = null, var firebaseAnalytics: Boolean? = null, var restoreTTLFilter: Boolean? = null, var clearGroupOnSummaryClick: Boolean? = null, var receiveReceiptEnabled: Boolean? = null, var disableGMSMissingPrompt: Boolean? = null, var unsubscribeWhenNotificationsDisabled: Boolean? = null, var locationShared: Boolean? = null, var requiresUserPrivacyConsent: Boolean? = null, var opRepoExecutionInterval: Long? = null, var influenceParams: InfluenceParamsObject, var fcmParams: FCMParamsObject, )</ID>
<ID>LongParameterList:IUserBackendService.kt$IUserBackendService$( appId: String, aliasLabel: String, aliasValue: String, properties: PropertiesObject, refreshDeviceMetadata: Boolean, propertyiesDelta: PropertiesDeltasObject, jwt: String? = null, )</ID>
<ID>LongParameterList:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$( private val _identityOperationExecutor: IdentityOperationExecutor, private val _application: IApplicationService, private val _deviceService: IDeviceService, private val _userBackend: IUserBackendService, private val _identityModelStore: IdentityModelStore, private val _propertiesModelStore: PropertiesModelStore, private val _subscriptionsModelStore: SubscriptionModelStore, private val _configModelStore: ConfigModelStore, private val _languageContext: ILanguageContext, private val _jwtTokenStore: JwtTokenStore, )</ID>
<ID>LongParameterList:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$( private val _identityOperationExecutor: IdentityOperationExecutor, private val _application: IApplicationService, private val _deviceService: IDeviceService, private val _userBackend: IUserBackendService, private val _identityModelStore: IdentityModelStore, private val _propertiesModelStore: PropertiesModelStore, private val _subscriptionsModelStore: SubscriptionModelStore, private val _configModelStore: ConfigModelStore, private val _languageContext: ILanguageContext, private val _jwtTokenStore: JwtTokenStore, private val _consistencyManager: IConsistencyManager, )</ID>
<ID>LongParameterList:OutcomeEventsController.kt$OutcomeEventsController$( private val _session: ISessionService, private val _influenceManager: IInfluenceManager, private val _outcomeEventsCache: IOutcomeEventsRepository, private val _outcomeEventsPreferences: IOutcomeEventsPreferences, private val _outcomeEventsBackend: IOutcomeEventsBackendService, private val _configModelStore: ConfigModelStore, private val _identityModelStore: IdentityModelStore, private val _subscriptionManager: ISubscriptionManager, private val _deviceService: IDeviceService, private val _time: ITime, )</ID>
<ID>LongParameterList:RefreshUserOperationExecutor.kt$RefreshUserOperationExecutor$( private val _userBackend: IUserBackendService, private val _identityModelStore: IdentityModelStore, private val _propertiesModelStore: PropertiesModelStore, private val _subscriptionsModelStore: SubscriptionModelStore, private val _configModelStore: ConfigModelStore, private val _buildUserService: IRebuildUserService, private val _newRecordState: NewRecordsState, private val _jwtTokenStore: JwtTokenStore, )</ID>
<ID>LongParameterList:SubscriptionObject.kt$SubscriptionObject$( val id: String? = null, val type: SubscriptionObjectType? = null, val token: String? = null, val enabled: Boolean? = null, val notificationTypes: Int? = null, val sdk: String? = null, val deviceModel: String? = null, val deviceOS: String? = null, val rooted: Boolean? = null, val netType: Int? = null, val carrier: String? = null, val appVersion: String? = null, )</ID>
Expand Down
18 changes: 10 additions & 8 deletions .../onesignal/core/src/main/java/com/onesignal/common/consistency/impl/ConsistencyManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,21 @@
}

override suspend fun resolveConditionsWithID(id: String) {
val completedConditions = mutableListOf<Pair<ICondition, CompletableDeferred<RywData?>>>()
mutex.withLock {
val completedConditions = mutableListOf<Pair<ICondition, CompletableDeferred<RywData?>>>()

for ((condition, deferred) in conditions) {
if (condition.id == id) {
if (!deferred.isCompleted) {
deferred.complete(null)
for ((condition, deferred) in conditions) {
if (condition.id == id) {
if (!deferred.isCompleted) {
deferred.complete(null)
}
}
completedConditions.add(Pair(condition, deferred))
}
completedConditions.add(Pair(condition, deferred))
}

// Remove completed conditions from the list
conditions.removeAll(completedConditions)
conditions.removeAll(completedConditions)
}

Check notice on line 73 in OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/consistency/impl/ConsistencyManager.kt

View check run for this annotation

Claude / Claude Code Review

resolveConditionsWithID removes ALL conditions regardless of ID match 

In `ConsistencyManager.resolveConditionsWithID()`, the line `completedConditions.add(Pair(condition, deferred))` at line 68 is placed OUTSIDE the `if (condition.id == id)` block (which closes at line 67), so every condition in the list—regardless of ID match—is added to `completedConditions` and then removed by `conditions.removeAll(completedConditions)`. This is a pre-existing logic error that the PR wrapped with `mutex.withLock` but did not fix; the PR adds a new call site at `LoginUserOperati
Comment on lines +62 to +73
Copy link
Copy Markdown

@claude claude bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 In ConsistencyManager.resolveConditionsWithID(), the line completedConditions.add(Pair(condition, deferred)) at line 68 is placed OUTSIDE the if (condition.id == id) block (which closes at line 67), so every condition in the list—regardless of ID match—is added to completedConditions and then removed by conditions.removeAll(completedConditions). This is a pre-existing logic error that the PR wrapped with mutex.withLock but did not fix; the PR adds a new call site at LoginUserOperationExecutor.kt:234 that increases exposure.

Extended reasoning...

What the bug is and how it manifests

In ConsistencyManager.resolveConditionsWithID() (lines 58–74), the statement completedConditions.add(Pair(condition, deferred)) at line 68 sits outside the if (condition.id == id) block. That if block ends at line 67 (closing brace). As a result, every iteration of the loop—regardless of whether condition.id matches—appends to completedConditions, and the subsequent conditions.removeAll(completedConditions) removes every registered condition, not just those matching id.

The specific code path that triggers it

Calling resolveConditionsWithID("IamFetchReadyCondition") when two conditions with different IDs are registered (e.g., type A with ID "IamFetchReadyCondition" and type B with a different static ID) would: (1) complete type A's deferred with null (correct), (2) silently remove type B from the list WITHOUT completing its deferred (incorrect). Type B's coroutine remains suspended on .await() indefinitely—the condition can never be fulfilled because it no longer exists in the list to be checked.

Why existing code does not prevent it

The indentation in the diff makes it look as though completedConditions.add() is inside the if block, but it is not—there is no closing brace before it for the if. The contrast with checkConditionsAndComplete() (line 88) makes the intent clear: that method correctly places completedConditions.add() inside if (condition.isMet(indexedTokens)), only accumulating entries that actually satisfied the predicate. The PR adds a mutex.withLock wrapper (fixing the data-race issue separately reported) but leaves the misplaced add() unchanged.

What the impact would be

In the current codebase all registered conditions are IamFetchReadyCondition instances sharing a single static ID constant (IamFetchReadyCondition.ID = "IamFetchReadyCondition"), so every entry matches the if check anyway and the bug is a no-op today. However, if any second condition type with a different static ID is ever registered and resolveConditionsWithID(typeA_ID) is called, type B conditions are silently removed with their deferreds uncompleted, permanently hanging any coroutine that called getRywDataFromAwaitableCondition() for a type B condition. The PR adds a new call site in LoginUserOperationExecutor.kt:234, increasing the surface area and making the latent bug more likely to manifest as the SDK's consistency manager usage grows.

Step-by-step proof

  1. Type A condition (id = "IamFetchReadyCondition") and type B condition (id = "SomeOtherCondition") are both registered; each creates a CompletableDeferred. conditions list has two entries.
  2. resolveConditionsWithID("IamFetchReadyCondition") is called.
  3. Loop iteration 1: condition.id == "IamFetchReadyCondition" → true; deferred A is completed with null. Then completedConditions.add() appends entry A (correct).
  4. Loop iteration 2: condition.id == "IamFetchReadyCondition" → false (type B); deferred B is NOT completed. Then completedConditions.add() appends entry B anyway (incorrect).
  5. conditions.removeAll(completedConditions) removes both A and B from the live list.
  6. Coroutine awaiting deferred B remains suspended forever; it cannot be resolved because B no longer exists in conditions to be matched by any future setRywData or resolveConditionsWithID call.

How to fix it

Move completedConditions.add(Pair(condition, deferred)) inside the if (condition.id == id) block, mirroring the pattern in checkConditionsAndComplete():

for ((condition, deferred) in conditions) {
    if (condition.id == id) {
        if (\!deferred.isCompleted) {
            deferred.complete(null)
        }
        completedConditions.add(Pair(condition, deferred))  // inside the if
    }
}

}

/**
Expand Down
27 changes: 16 additions & 11 deletions ...core/src/main/java/com/onesignal/core/internal/config/impl/IdentityVerificationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.onesignal.core.internal.config.impl
import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
import com.onesignal.common.modeling.ModelChangeTags
import com.onesignal.common.modeling.ModelChangedArgs
import com.onesignal.common.threading.suspendifyOnIO
import com.onesignal.core.internal.config.ConfigModel
import com.onesignal.core.internal.config.ConfigModelStore
import com.onesignal.core.internal.operations.IOperationRepo
Expand Down Expand Up @@ -42,21 +43,25 @@ internal class IdentityVerificationService(

val useIV = model.useIdentityVerification

var jwtInvalidatedExternalId: String? = null
if (useIV == true) {
Logging.debug("IdentityVerificationService: IV enabled, purging anonymous operations")
_operationRepo.removeOperationsWithoutExternalId()
suspendifyOnIO {
_operationRepo.awaitInitialized()

val externalId = _identityModelStore.model.externalId
if (externalId != null && _jwtTokenStore.getJwt(externalId) == null) {
Logging.debug("IdentityVerificationService: IV enabled but no JWT for $externalId, will fire invalidated event after queue wake")
jwtInvalidatedExternalId = externalId
var jwtInvalidatedExternalId: String? = null
if (useIV == true) {
Logging.debug("IdentityVerificationService: IV enabled, purging anonymous operations")
_operationRepo.removeOperationsWithoutExternalId()

val externalId = _identityModelStore.model.externalId
if (externalId != null && _jwtTokenStore.getJwt(externalId) == null) {
Logging.debug("IdentityVerificationService: IV enabled but no JWT for $externalId, will fire invalidated event after queue wake")
jwtInvalidatedExternalId = externalId
}
}
}

_operationRepo.forceExecuteOperations()
_operationRepo.forceExecuteOperations()

jwtInvalidatedExternalId?.let { _userManager.fireJwtInvalidated(it) }
jwtInvalidatedExternalId?.let { _userManager.fireJwtInvalidated(it) }
}
}

override fun onModelUpdated(
Expand Down
4 changes: 1 addition & 3 deletions ...onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,7 @@ internal class OperationRepo(
_operationModelStore.remove(it.operation.id)
it.waiter?.wake(false)
}
if (toRemove.isNotEmpty()) {
Logging.debug("OperationRepo: removed ${toRemove.size} anonymous operations (no externalId)")
}
Logging.debug("OperationRepo: removeOperationsWithoutExternalId removed ${toRemove.size} of ${toRemove.size + queue.size} operations")
}
}

Expand Down
37 changes: 28 additions & 9 deletions OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,45 @@ internal open class UserManager(
private val jwtInvalidatedAppCallbackScope =
CoroutineScope(SupervisorJob() + Dispatchers.Default)

private val jwtInvalidatedLock = Any()
private var pendingJwtInvalidatedExternalId: String? = null

fun addJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) {
jwtInvalidatedNotifier.subscribe(listener)
val pendingExternalId: String?
synchronized(jwtInvalidatedLock) {
jwtInvalidatedNotifier.subscribe(listener)
pendingExternalId = pendingJwtInvalidatedExternalId
pendingJwtInvalidatedExternalId = null
}
pendingExternalId?.let {
listener.onUserJwtInvalidated(UserJwtInvalidatedEvent(it))
}
}

fun removeJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) {
jwtInvalidatedNotifier.unsubscribe(listener)
}

/**
* Schedules [IUserJwtInvalidatedListener] delivery on a background dispatcher so HYDRATE and
* operation-repo paths can finish internal work before app code runs.
* Fires [IUserJwtInvalidatedListener] to all subscribers asynchronously so the caller
* (e.g. OperationRepo) is not blocked by developer code. If no listeners are registered yet
* (e.g. during SDK init), stores the externalId so it can be replayed when the first
* listener is added via [addJwtInvalidatedListener].
*/
fun fireJwtInvalidated(externalId: String) {
jwtInvalidatedAppCallbackScope.launch {
runCatching {
jwtInvalidatedNotifier.fire { listener ->
listener.onUserJwtInvalidated(UserJwtInvalidatedEvent(externalId))
synchronized(jwtInvalidatedLock) {
if (jwtInvalidatedNotifier.hasSubscribers) {
jwtInvalidatedAppCallbackScope.launch {
runCatching {
jwtInvalidatedNotifier.fire { listener ->
listener.onUserJwtInvalidated(UserJwtInvalidatedEvent(externalId))
}
}.onFailure {
Logging.warn("Failed to deliver JWT invalidated event for externalId=$externalId", it)
}
}
}.onFailure {
Logging.warn("Failed to deliver JWT invalidated event for externalId=$externalId", it)
} else {
pendingJwtInvalidatedExternalId = externalId
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions ...K/onesignal/core/src/main/java/com/onesignal/user/internal/backend/IUserBackendService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,8 @@ class CreateUserResponse(
* The subscriptions for the user.
*/
val subscriptions: List<SubscriptionObject>,
/**
* Read-your-write consistency data returned by the backend, if any.
*/
val rywData: RywData? = null,
)
8 changes: 7 additions & 1 deletion ...DK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/JSONConverter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.onesignal.user.internal.backend.impl

import com.onesignal.common.consistency.RywData
import com.onesignal.common.expandJSONArray
import com.onesignal.common.putJSONArray
import com.onesignal.common.putMap
Expand All @@ -8,6 +9,7 @@ import com.onesignal.common.safeBool
import com.onesignal.common.safeDouble
import com.onesignal.common.safeInt
import com.onesignal.common.safeJSONObject
import com.onesignal.common.safeLong
import com.onesignal.common.safeString
import com.onesignal.common.toMap
import com.onesignal.user.internal.backend.CreateUserResponse
Expand Down Expand Up @@ -55,7 +57,11 @@ object JSONConverter {
return@expandJSONArray null
}

return CreateUserResponse(respIdentities, respProperties, respSubscriptions)
val rywToken = jsonObject.safeString("ryw_token")
val rywDelay = jsonObject.safeLong("ryw_delay")
val rywData = if (rywToken != null) RywData(rywToken, rywDelay) else null

return CreateUserResponse(respIdentities, respProperties, respSubscriptions, rywData)
}

fun convertToJSON(properties: PropertiesObject): JSONObject {
Expand Down
21 changes: 21 additions & 0 deletions ...nalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/identity/JwtTokenStore.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
package com.onesignal.user.internal.identity

import com.onesignal.common.events.EventProducer
import com.onesignal.core.internal.preferences.IPreferencesService
import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys
import com.onesignal.core.internal.preferences.PreferenceStores
import com.onesignal.debug.internal.logging.Logging
import org.json.JSONException
import org.json.JSONObject

/**
* Listener notified when a JWT is stored or replaced for an external ID.
*/
fun interface IJwtUpdateListener {
/** Called after [JwtTokenStore.putJwt] persists a new token for [externalId]. */
fun onJwtUpdated(externalId: String)
}

/**
* Persistent store mapping externalId -> JWT token. Supports multiple users simultaneously
* so that queued operations for a previous user can still resolve their JWT at execution time.
Expand All @@ -20,6 +29,7 @@ class JwtTokenStore(
) {
private val tokens: MutableMap<String, String> = mutableMapOf()
private var isLoaded = false
private val jwtUpdateNotifier = EventProducer<IJwtUpdateListener>()

/** Not thread-safe; callers must hold `synchronized(tokens)`. */
private fun ensureLoaded() {
Expand Down Expand Up @@ -61,6 +71,16 @@ class JwtTokenStore(
}
}

/** Register a [listener] to be notified when any JWT is updated via [putJwt]. */
fun subscribe(listener: IJwtUpdateListener) {
jwtUpdateNotifier.subscribe(listener)
}

/** Remove a previously registered [listener]. */
fun unsubscribe(listener: IJwtUpdateListener) {
jwtUpdateNotifier.unsubscribe(listener)
}

/**
* Stores (or replaces) the JWT for [externalId]. Passing a null [jwt] is a no-op;
* use [invalidateJwt] to remove a token.
Expand All @@ -75,6 +95,7 @@ class JwtTokenStore(
tokens[externalId] = jwt
persist()
}
jwtUpdateNotifier.fire { it.onJwtUpdated(externalId) }
}

/**
Expand Down
10 changes: 10 additions & 0 deletions .../java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import com.onesignal.common.NetworkUtils
import com.onesignal.common.OneSignalUtils
import com.onesignal.common.RootToolsInternalMethods
import com.onesignal.common.TimeUtils
import com.onesignal.common.consistency.IamFetchReadyCondition
import com.onesignal.common.consistency.enums.IamFetchRywTokenKey
import com.onesignal.common.consistency.models.IConsistencyManager
import com.onesignal.common.exceptions.BackendException
import com.onesignal.common.modeling.ModelChangeTags
import com.onesignal.core.internal.application.IApplicationService
Expand Down Expand Up @@ -49,6 +52,7 @@ internal class LoginUserOperationExecutor(
private val _configModelStore: ConfigModelStore,
private val _languageContext: ILanguageContext,
private val _jwtTokenStore: JwtTokenStore,
private val _consistencyManager: IConsistencyManager,
) : IOperationExecutor {
override val operations: List<String>
get() = listOf(LOGIN_USER)
Expand Down Expand Up @@ -224,6 +228,12 @@ internal class LoginUserOperationExecutor(
backendSubscriptions.remove(backendSubscription)
}

if (response.rywData != null) {
_consistencyManager.setRywData(backendOneSignalId, IamFetchRywTokenKey.USER, response.rywData)
} else {
_consistencyManager.resolveConditionsWithID(IamFetchReadyCondition.ID)
}

val wasPossiblyAnUpsert = identities.isNotEmpty()
val followUpOperations =
if (wasPossiblyAnUpsert) {
Expand Down
Loading
Loading