Conversation
…ication nullable, OptionalHeaders.jwt Foundational models and infrastructure for identity verification: - Create JwtTokenStore: persistent Map<externalId, JWT> backed by SharedPreferences, supporting multi-user JWT storage with getJwt/putJwt/invalidateJwt/pruneToExternalIds - Add var externalId to Operation base class so OperationRepo can stamp and gate operations per-user; remove redundant externalId from LoginUserOperation and TrackCustomEventOperation (same Model data-map key, no migration needed) - Change ConfigModel.useIdentityVerification from Boolean to Boolean? (null = unknown, false = off, true = on) to eliminate race between operation processing and remote params - Add jwt field to OptionalHeaders for passing Bearer tokens through HTTP layer - Add PREFS_OS_JWT_TOKENS key to PreferenceOneSignalKeys Made-with: Cursor
…ion Bearer header to HttpClient Identity verification: plumb JWT through the HTTP and backend layer. - HttpClient: set Authorization: Bearer header when OptionalHeaders.jwt is non-null - IIdentityBackendService + impl: add jwt param to setAlias, deleteAlias - ISubscriptionBackendService + impl: add jwt param to createSubscription, updateSubscription, deleteSubscription, transferSubscription, getIdentityFromSubscription - IUserBackendService + impl: add jwt param to createUser, updateUser, getUser - All jwt params default to null so existing callers are unaffected
…D handling to OperationRepo Identity verification: OperationRepo becomes JWT-aware. - Add JwtTokenStore and IdentityModelStore as constructor dependencies - Centralized externalId stamping: internalEnqueue() auto-sets op.externalId from the current identity model for new operations (not loaded from persistence, not already set by the operation's constructor) - IV gating in getNextOps(): when IV=null (unknown), hold ALL operations; when IV=true, skip operations without a valid JWT; when IV=false, proceed normally - FAIL_UNAUTHORIZED: invalidate the per-user JWT in JwtTokenStore and re-queue operations to front (held by JWT gating until a new JWT is provided) - Cold-start cleanup: prune JwtTokenStore to externalIds from pending operations plus the current identity model's externalId - New removeOperationsWithoutExternalId() method on IOperationRepo for IdentityVerificationService to purge anonymous operations when IV is enabled
…tity verification - Add resolveIdentityAlias() helper to dynamically choose external_id vs onesignal_id for API paths - Each executor now looks up JWT from JwtTokenStore using operation's externalId and passes it to backend calls - LoginUserFromSubscriptionOperationExecutor returns FAIL_NORETRY when IV is enabled (v4 migration safety net) Made-with: Cursor
- Add jwt param to IInAppBackendService.listInAppMessages and pass through OptionalHeaders - Skip IAM fetch when identity verification is enabled and user is anonymous - Look up JWT from JwtTokenStore for authenticated IAM requests Made-with: Cursor
…subscriptions/:subscription_id/iams Made-with: Cursor
… and subscription listeners - LoginHelper: store JWT unconditionally on login, handle same-externalId re-login (store + forceExecute), set existingOneSignalId to null when IV=ON - LogoutHelper: IV=ON branch opts out push before user switch so backend is notified, then creates local-only anonymous user with suppressBackendOperation - UserManager: add jwtInvalidatedNotifier EventProducer for JWT callbacks - OneSignalImp: wire updateUserJwt, addUserJwtInvalidatedListener, removeUserJwtInvalidatedListener; pass JwtTokenStore/SubscriptionModelStore to LoginHelper/LogoutHelper - SubscriptionModelStoreListener, IdentityModelStoreListener, PropertiesModelStoreListener: suppress ops for anonymous users when IV=ON Made-with: Cursor
- New IdentityVerificationService: listens for config HYDRATE events, purges anonymous operations when IV=true, wakes OperationRepo when IV resolves from null, fires UserJwtInvalidatedEvent for beta migration (externalId present but no JWT) - CoreModule: register JwtTokenStore and IdentityVerificationService - ParamsBackendService: remove leftover TODO comments Made-with: Cursor
Register for JWT invalidation events, log a warning and show a toast when triggered.
Optional headers (ETag, RYW-Token, Retry-Count, Session-Duration, Authorization) were set after the body write, which opens the connection. This caused IllegalStateException on POST requests with a JWT. Move all setRequestProperty calls before outputStream write to prevent the error. Made-with: Cursor
Cache the JWT token on login and updateUserJwt calls. When Identity Verification is enabled, include the Authorization Bearer header in the demo app's fetch user request.
When Identity Verification is ON and logout is called, the SDK now sets isDisabledInternally=true instead of optedIn=false. This preserves the real opt-in preference while telling the backend the subscription is disabled. On the next login, UserSwitcher creates a fresh SubscriptionModel that defaults isDisabledInternally=false, restoring the real state automatically. Made-with: Cursor
Add addJwtInvalidatedListener, removeJwtInvalidatedListener, and fireJwtInvalidated methods to UserManager. Callers no longer need to cast IUserManager to UserManager to access the notifier directly. Register UserManager as a concrete DI service so IdentityVerificationService can depend on it directly. Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Add KDoc to public JWT API functions, update detekt baseline
4509a8f to
457b745
Compare
| ExecutionResult.FAIL_UNAUTHORIZED -> { | ||
| val externalId = startingOp.operation.externalId | ||
| if (externalId != null) { | ||
| _jwtTokenStore.invalidateJwt(externalId) | ||
| Logging.warn("Operation execution failed with 401 Unauthorized, JWT invalidated for user: $externalId. Operations re-queued.") | ||
| synchronized(queue) { | ||
| ops.reversed().forEach { queue.add(0, it) } | ||
| } | ||
| } else { | ||
| Logging.warn("Operation execution failed with 401 Unauthorized for anonymous user. Operations dropped.") | ||
| ops.forEach { _operationModelStore.remove(it.operation.id) } | ||
| ops.forEach { it.waiter?.wake(false) } | ||
| } |
There was a problem hiding this comment.
🔴 When a runtime API call returns 401, OperationRepo.executeOperations() invalidates the JWT and re-queues the operations, but never calls UserManager.fireJwtInvalidated() — so the developer's IUserJwtInvalidatedListener is never invoked. Because hasValidJwtIfRequired() now returns false for the re-queued operations, getNextOps permanently skips them and the queue is deadlocked until the app is cold-restarted.
Extended reasoning...
What the bug is and how it manifests
When Identity Verification (IV) is enabled and the backend returns HTTP 401, OperationRepo.executeOperations() handles ExecutionResult.FAIL_UNAUTHORIZED (lines 283–295 of OperationRepo.kt). It calls _jwtTokenStore.invalidateJwt(externalId) to mark the token as expired, then re-inserts all failed operations at the front of the queue. The developer is never notified that a new JWT is needed. The operations re-queued are now permanently gated by hasValidJwtIfRequired(), which returns false because the JWT was just removed from the store. getNextOps() will silently skip these operations on every pass, and the loop returns to waitForNewOperationAndExecutionInterval(), waiting indefinitely for a new operation that will never come.
The specific code path that triggers it
UserManager.fireJwtInvalidated() is called from exactly one location: IdentityVerificationService.onModelReplaced() (line 49 of IdentityVerificationService.kt), which only fires when the config model is replaced with tag ModelChangeTags.HYDRATE — i.e., during cold app startup. OperationRepo receives _jwtTokenStore and _identityModelStore as constructor parameters but has no reference to UserManager, so it has no mechanism to fire the event.
Why existing code doesn't prevent it
The IUserJwtInvalidatedListener public API was designed exactly to handle this case — the developer subscribes, receives the callback, calls OneSignal.updateUserJwt(externalId, newToken), and the queue unblocks. However, the only wire-up of this mechanism (IdentityVerificationService) only runs once at startup, not on runtime 401 errors. The FAIL_UNAUTHORIZED branch in OperationRepo was written with the JWT store side-effect but without the notification side-effect.
Step-by-step proof
- App starts, IV is enabled (
useIdentityVerification == true). Developer provides a JWT vialogin(externalId, jwt).JwtTokenStorestores the token. - Some time later the JWT expires on the server. An operation (e.g.,
UpdateSubscriptionOperation) is dequeued and executed. The backend returns 401. executeOperations()receivesExecutionResult.FAIL_UNAUTHORIZED. The code at line 286 calls_jwtTokenStore.invalidateJwt(externalId)— the token is now gone from the store. The operations are put back at index 0 of the queue.processQueueForevercallsgetNextOps(executeBucket). InsidegetNextOps,hasValidJwtIfRequired(iv=true, op)is called for each queue item. Because_jwtTokenStore.getJwt(externalId)returnsnull, the function returnsfalse.startingOpis alwaysnull.getNextOpsreturnsnull. The loop falls intowaitForNewOperationAndExecutionInterval()and blocks.- The developer's
IUserJwtInvalidatedListener.onUserJwtInvalidated()is never called becausefireJwtInvalidatedis never invoked fromOperationRepo. The developer has no idea a new token is required. The queue stays stuck until the app process is killed and restarted.
Impact
Any app with IV enabled will silently lose all pending operations after a runtime JWT expiry. Push subscription updates, tag changes, alias operations, custom events, etc., will pile up in the queue and never execute. The developer cannot recover programmatically without a cold restart, defeating the entire purpose of IUserJwtInvalidatedListener.
How to fix it
Inject a (String) -> Unit callback (or the UserManager itself) into OperationRepo alongside _jwtTokenStore. In the FAIL_UNAUTHORIZED branch, after _jwtTokenStore.invalidateJwt(externalId), call onJwtInvalidated(externalId) (which delegates to userManager.fireJwtInvalidated(externalId)). This mirrors how IdentityVerificationService already handles the startup case.
|
|
||
| // Create new device-scoped user (clears external ID) | ||
| userSwitcher.createAndSwitchToNewUser() | ||
| if (configModel.useIdentityVerification == true) { | ||
| configModel.pushSubscriptionId?.let { pushSubId -> | ||
| subscriptionModelStore.get(pushSubId) | ||
| ?.let { it.isDisabledInternally = true } | ||
| } | ||
|
|
||
| // Enqueue login operation for the new device-scoped user (no external ID) | ||
| operationRepo.enqueue( | ||
| LoginUserOperation( | ||
| configModel.appId, | ||
| identityModelStore.model.onesignalId, | ||
| null, | ||
| // No external ID for device-scoped user | ||
| ), | ||
| ) | ||
| userSwitcher.createAndSwitchToNewUser(suppressBackendOperation = true) | ||
| } else { | ||
| userSwitcher.createAndSwitchToNewUser() | ||
|
|
||
| // TODO: remove JWT Token for all future requests. | ||
| operationRepo.enqueue( | ||
| LoginUserOperation( | ||
| configModel.appId, |
There was a problem hiding this comment.
🔴 When logging out with Identity Verification enabled, the UpdateSubscriptionOperation that disables the push subscription is stamped with a null externalId due to an async/sync ordering race, causing hasValidJwtIfRequired() to permanently block the operation. The backend subscription is never disabled, so the logged-out user continues receiving push notifications — a privacy/security issue.
Extended reasoning...
Root cause — async stamp races synchronous user switch
In LogoutHelper.logout() (within synchronized(lock)), the IV=true branch first sets subscriptionModel.isDisabledInternally = true (LogoutHelper.kt line 25-26), then calls createAndSwitchToNewUser(suppressBackendOperation = true) (line 29). Setting isDisabledInternally fires SubscriptionModelStoreListener.getUpdateOperation() synchronously via the model property-change infrastructure. At this moment the identity model still has the logged-in user's externalId, so shouldSuppressForAnonymousUser() returns false and an UpdateSubscriptionOperation is created and passed to operationRepo.enqueue(op).
The async gap
OperationRepo.enqueue() does not stamp externalId immediately — it dispatches scope.launch { internalEnqueue(...) } onto OSOperationRepoScope (OperationRepo.kt around line 136). The externalId stamping logic in internalEnqueue() runs: . That coroutine is queued but has not yet run.
The overwrite
Still inside synchronized(lock), createAndSwitchToNewUser(suppressBackendOperation=true) runs synchronously on the calling thread. This calls UserSwitcher/identityModelStore.replace() with a fresh anonymous IdentityModel (externalId = null). The lock is then released.
Stamp occurs too late
Only after the lock is released does internalEnqueue run on OSOperationRepoScope. By then _identityModelStore.model.externalId is null (the new anonymous user). The UpdateSubscriptionOperation is stamped with externalId = null.
Permanent blockage
In getNextOps(), hasValidJwtIfRequired() evaluates: . Because externalId is null, this returns false forever. With IV = true and no way for the operation to acquire a valid externalId, it sits in the queue indefinitely and never executes. The backend PATCH to disabled=true / UNSUBSCRIBE is never sent.
Concrete proof
- User logs in as user@example.com with a valid JWT; push subscription is active on the backend.
- App calls OneSignal.logout() while useIdentityVerification == true.
- Inside synchronized(lock): subscriptionModel.isDisabledInternally = true → SubscriptionModelStoreListener fires synchronously → UpdateSubscriptionOperation{externalId=null(not yet set), enabled=false, status=UNSUBSCRIBE} is passed to enqueue() → scope.launch{internalEnqueue(…)} is scheduled but not yet run.
- Still inside lock: createAndSwitchToNewUser(suppressBackendOperation=true) → identityModelStore.model.externalId = null (anonymous user).
- Lock released. internalEnqueue runs: stamps op.externalId = null.
- getNextOps() → hasValidJwtIfRequired(iv=true, op) → op.externalId ?: return false → returns false → operation blocked forever.
- Backend still has subscription enabled=true. Notification delivery continues for the now-logged-out user.
Fix
The externalId should be captured and stamped synchronously before enqueue() dispatches to the scope, or the UpdateSubscriptionOperation should be created and explicitly given the old user's externalId before createAndSwitchToNewUser() runs. One clean approach: capture the externalId before setting isDisabledInternally, construct the UpdateSubscriptionOperation explicitly with that externalId, and enqueue it before switching users — or stamp it in the same synchronized block inside the operation object prior to the scope.launch.
| private fun ensureLoaded() { | ||
| if (isLoaded) return | ||
| val json = | ||
| _prefs.getString( | ||
| PreferenceStores.ONESIGNAL, | ||
| PreferenceOneSignalKeys.PREFS_OS_JWT_TOKENS, | ||
| ) | ||
| if (json != null) { | ||
| val obj = JSONObject(json) | ||
| for (key in obj.keys()) { | ||
| tokens[key] = obj.getString(key) | ||
| } | ||
| } | ||
| isLoaded = true | ||
| } |
There was a problem hiding this comment.
🔴 In JwtTokenStore.ensureLoaded(), JSONObject(json) on line 31 is not wrapped in a try-catch, so malformed SharedPreferences data (e.g. from an OS crash mid-write) throws an uncaught JSONException. Because isLoaded = true is only set after successful parsing, the flag stays false and every subsequent call to getJwt/putJwt/invalidateJwt/pruneToExternalIds re-enters ensureLoaded and throws again, creating an infinite failure loop for the entire process lifetime. The fix is to wrap the JSON parsing in try/catch and set isLoaded = true even on failure, treating corrupted data as an empty token map.
Extended reasoning...
What the bug is and how it manifests
JwtTokenStore.ensureLoaded() (lines 23-37) reads the persisted JWT map from SharedPreferences and parses it with JSONObject(json) (line 31). There is no try-catch around this call. If the stored string is malformed JSON, org.json.JSONException is thrown and propagates uncaught out of ensureLoaded(). Critically, isLoaded = true (line 36) is located after the parsing block and is therefore never executed on failure. On the very next call to any public method—getJwt, putJwt, invalidateJwt, or pruneToExternalIds—the synchronized block calls ensureLoaded() again (because isLoaded is still false), throws again, and this cycle repeats for the entire process lifetime.
The specific code path that triggers it
- App starts; OperationRepo.loadSavedOperations() eventually calls _jwtTokenStore.pruneToExternalIds(...).
- pruneToExternalIds() acquires synchronized(tokens) and calls ensureLoaded().
- ensureLoaded() reads the SharedPreferences value, finds a non-null but malformed JSON string, and calls JSONObject(json).
- JSONException is thrown; isLoaded remains false; the exception propagates through pruneToExternalIds() and up the call stack.
- Any later call (e.g. getJwt inside hasValidJwtIfRequired, or putJwt inside login) hits the same path and throws again.
Why existing code doesn't prevent it
SharedPreferences.getString() returns the raw stored string without validating it. The only writer of PREFS_OS_JWT_TOKENS is persist(), which always produces valid JSON via JSONObject(tokens.toMap()).toString(). However, an OS-level crash or power loss mid-commit can leave the underlying XML file partially written. Android's SharedPreferences uses an atomic rename approach (write to .bak then rename), but a kill between the write and the rename, or filesystem-level corruption, can still produce a state the parser cannot handle. Additionally, PREFS_OS_JWT_TOKENS is a new key introduced by this PR, so existing devices won't have stale data—but the risk materialises from the first write onwards.
What the impact would be
When IV (identity verification) is enabled and this failure occurs, every JWT operation throws indefinitely. getNextOps() calls hasValidJwtIfRequired() which calls getJwt() which throws, halting the operation queue. putJwt() (called from login and updateUserJwt) also throws, so a fresh JWT can never be stored. The SDK effectively becomes non-functional for all network operations for that process lifetime.
How to fix it
Wrap the JSON parsing block in try/catch(Exception) and ensure isLoaded = true is reached regardless:
Step-by-step proof
- Device has previously called putJwt("user1", "eyJ..."), which wrote valid JSON to PREFS_OS_JWT_TOKENS.
- Mid-write during a subsequent putJwt call, the OS kills the process (e.g. low-memory). The .bak file is now a partial XML fragment.
- On next app start, PreferencesService reads the partial file and returns the corrupted string (e.g. "{"user1":"eyJ") from getString().
- ensureLoaded() calls JSONObject("{"user1":"eyJ") → JSONException: Unterminated string at character 20.
- isLoaded stays false. The exception propagates.
- OperationRepo.loadSavedOperations() fails; initialized.complete(Unit) is never called.
- All callers of awaitInitialized() block forever; all JWT lookups in getNextOps() throw; the operation queue is permanently stalled.
| } | ||
|
|
||
| internal fun getNextOps(bucketFilter: Int): List<OperationQueueItem>? { | ||
| val iv = _configModelStore.model.useIdentityVerification | ||
| if (iv == null) return null | ||
|
|
There was a problem hiding this comment.
🔴 When the backend response omits the jwt_required field (i.e. for all apps without Identity Verification enabled), getNextOps() returns null permanently, silently blocking every SDK operation for the entire session. The fix is to default useIdentityVerification to false when the field is absent from the params response rather than leaving it null.
Extended reasoning...
The bug
OperationRepo.getNextOps() (line 397-401) was modified by this PR to gate execution on the value of ConfigModel.useIdentityVerification:
val iv = _configModelStore.model.useIdentityVerification
if (iv == null) return nullThe intent is that null means "remote params haven't been fetched yet, hold all operations." Once params arrive (via HYDRATE), IdentityVerificationService.onModelReplaced() calls forceExecuteOperations() to release the hold. This design works only if HYDRATE always sets useIdentityVerification to a non-null value.
Why it stays null
ParamsBackendService reads the value as:
useIdentityVerification = responseJson.safeBool("jwt_required")safeBool returns null when the field is absent from the JSON response. For any app that has not enabled Identity Verification on the server side, jwt_required is simply not present in the android_params response, so params.useIdentityVerification is null.
ConfigModelStoreListener then copies the fetched params into the config model using ?.let:
params.useIdentityVerification?.let { config.useIdentityVerification = it }Because the incoming value is null, the ?.let block is skipped entirely — config.useIdentityVerification is never set and remains null (its initial default from getOptBooleanProperty).
Why forceExecuteOperations() doesn't help
After HYDRATE, IdentityVerificationService.onModelReplaced() calls _operationRepo.forceExecuteOperations(), which wakes the sleeping coroutine in processQueueForever(). That coroutine then calls getNextOps() — which again reads iv == null and returns null. The loop falls back to waitForNewOperationAndExecutionInterval(). No further HYDRATE event will ever occur in the session, so the queue is permanently blocked.
Step-by-step proof for a non-IV app
- App starts;
config.useIdentityVerification = null(not yet set). processQueueForever()callsgetNextOps()→ returnsnull→ waits.ConfigModelStoreListenerfetchesandroid_params; server returns JSON withoutjwt_required.safeBool("jwt_required")→null;params.useIdentityVerification = null.params.useIdentityVerification?.let { ... }→ skipped;config.useIdentityVerificationstaysnull.- HYDRATE fires;
IdentityVerificationService.onModelReplaced()callsforceExecuteOperations(). - Woken coroutine calls
getNextOps()→iv == null→ returnsnull→ goes back to waiting. - No further HYDRATE occurs. Every enqueued operation (session tracking, push registration, tag updates, etc.) is silently held forever.
Impact
This is a complete regression from the pre-PR behavior where operations executed without any IV check. Every OneSignal customer whose app does not use Identity Verification — the vast majority — will experience zero analytics, no push subscription registration on first launch, and no user property sync for the entire session. The failure is silent from the app's perspective.
Fix
Default useIdentityVerification to false when jwt_required is absent. The simplest change is in ParamsBackendService:
useIdentityVerification = responseJson.safeBool("jwt_required") ?: falseAlternatively, ConfigModelStoreListener can assign params.useIdentityVerification ?: false instead of using ?.let. Either ensures that after HYDRATE the field is always non-null, allowing getNextOps() to proceed normally for non-IV apps.
bc22ea4 to
22fe4f9
Compare
Made-with: Cursor
22fe4f9 to
113e96b
Compare
READ AND DELETE THIS SECTION BEFORE SUBMITTING PR
Description
One Line Summary
REQUIRED - Very short description that summaries the changes in this PR.
Details
Motivation
REQUIRED - Why is this code change being made? Or what is the goal of this PR? Examples: Fixes a specific bug, provides additional logging to debug future issues, feature to allow X.
Scope
RECOMMEND - OPTIONAL - What is intended to be effected. What is known not to change. Example: Notifications are grouped when parameter X is set, not enabled by default.
OPTIONAL - Other
OPTIONAL - Feel free to add any other sections or sub-sections that can explain your PR better.
Testing
Unit testing
OPTIONAL - Explain unit tests added, if not clear in the code.
Manual testing
RECOMMEND - OPTIONAL - Explain what scenarios were tested and the environment.
Example: Tested opening a notification while the app was foregrounded, app build with Android Studio 2020.3 with a fresh install of the OneSignal example app on a Pixel 6 with Android 12.
Affected code checklist
Checklist
Overview
Testing
Final pass