Identity Verification (JWT) — full stack for adversarial review by nan-li · Pull Request #1708 · OneSignal/OneSignal-iOS-SDK

nan-li · 2026-08-12T01:35:51Z

Summary

Review-only umbrella PR. Diff is tip of the stacked JWT work (nan/jwt-pr8-demo-ui) against main, so adversarial review can see the whole Identity Verification / JWT change set in one place.

Not the merge vehicle for shipping IV — the reviewable stack targets 5.6-main / individual PRs (#1705–#1707+). Prefer commenting on those when possible; use this PR when you need cross-cutting context.

What’s in this diff (stacked slices)

Slice	Focus
Chore / consistency	Canonical `project.pbxproj` ordering; RYW consistency fixes (scope resolve to one user, lower subscription bar on wait timeout)
PR3 — IV gate	Feature flags, JWT config, Identity Verification gate
PR4 — public API	Identity model JWT, JWT repo, public user JWT API
PR5 — deltas	Bind each Delta to the user that owns it
PR6 — request pipeline	Auth / path encoding / JWT on user+subscription requests; related test holds
PR7 — IAM	In-app messages under Identity Verification
PR8 — demo	Demo app Identity Verification UI

Intent of the feature

External-ID–scoped JWTs so user-mutating and related traffic can be Identity-Verification gated, with ownership tracking so switches / races don’t apply the wrong user’s token or unblock the wrong waiter.

Test plan

Adversarial pass over gate, repo, public API, delta ownership, request auth, IAM, and demo wiring as one story
Flag cross-slice races (user switch, timeout vs resolve, purge under TEST delay, subscription bar)
Confirm nothing here assumes merge-to-main for 5.x GA — review feedback feeds the stacked PRs

Made with Cursor

Canonical sort applied by the xcodeproj tool, so later commits in this stack show only their own file additions. No project changes. Co-authored-by: Cursor <cursoragent@cursor.com>

Four faults in how a fetch waits for its own write to be readable: resolveConditionsWithID looked up waiters by condition id, but they are registered under the id passed to getRywTokenFromAwaitableCondition, so the lookup found nothing and the waiter it meant to release stayed blocked. It now scans every index for waiters on that condition. A waiter blocked on an unbounded semaphore, so a response that never arrived held the calling thread for the life of the process. Waits now time out and deregister rather than leaving an entry that the next token signals to nobody. OSIamFetchReadyCondition was a singleton pinned to the first id it ever saw, so after a user switch a fetch consulted the previous user's tokens. Conditions are now per id, with reset() as the test seam. hasSubscriptionUpdatePending was never lowered, so one in-session subscription change held every later fetch to waiting for a subscription token with no update behind it. The new optional onConditionSatisfied lets a condition lower a bar it raised once its waiter is released. Shared state moves behind the serial queue and locks throughout. Co-authored-by: Cursor <cursoragent@cursor.com>

resolveConditionsWithID walked every index and cleared every matching condition's subscription bar, so a response with no ryw_token for one user could unblock — and disarm — another. Resolve now takes the onesignal id and only touches that bucket. A timed-out waiter only deregistered itself, leaving hasSubscriptionUpdatePending up, so later IAM fetches for that id paid another full wait for a subscription token that was never coming. Timeout now runs the same onConditionSatisfied clear, if the waiter is still registered. Co-authored-by: Cursor <cursoragent@cursor.com>

Cut hazard essays and cross-user narration down to short whys hitched to the action, and trim the resolveConditions doc to the contract. Co-authored-by: Cursor <cursoragent@cursor.com>

…gate The foundation the rest of Identity Verification is built on. Nothing consumes the gate yet, so this changes no behavior beyond the params handling below. OSUserJwtConfig holds whether the app requires a token as a three-state answer — unknown until remote params say — because "not yet known" and "not required" have to be told apart before anything can be sent. OSFeatureManager carries the flags remote params enable, and OSIdentityVerificationService is the single place that answers whether Identity Verification is on. Remote params now hydrate that requirement from jwt_required. A response that omits the key means Identity Verification is off for the app, while an empty response answers nothing and leaves the cached requirement standing. Because the operation repo will hold every queued operation until the requirement is known, a params request that failed was going to cost a first-launch app every tag, session count and event for the rest of the session: nothing retried it. It now retries with a backoff over five attempts and, failing that, tries again on the next session. Replaces the unread require_user_id_auth key with jwt_required. The requiresUserAuth property it wrote is removed in the next PR. Co-authored-by: Cursor <cursoragent@cursor.com>

cursor

Multimodal adversarial review (interrogate)

Skill: Cursor interrogate (pstack). Reviewers: claude-fable-5-thinking-xhigh, gpt-5.6-sol-xhigh, cursor-grok-4.5-high-fast, claude-opus-5-thinking-high. Lead judgment applied below — feed findings into the stacked PRs (#1705–#1707+), not as a merge-to-main checklist.

Intent

External-ID–scoped JWTs so user-mutating and related SDK traffic can be Identity-Verification gated, with ownership tracking so switches / races don’t apply the wrong user’s token or unblock the wrong waiter. Umbrella stack: gate + JWT repo/public API + delta ownership + IV request pipeline + IAM under IV + RYW fixes + demo UI.

Reviewers

Reviewer A: claude-fable-5-thinking-xhigh, 10 findings
Reviewer B: gpt-5.6-sol-xhigh, 7 findings
Reviewer C: cursor-grok-4.5-high-fast, 9 findings
Reviewer D: claude-opus-5-thinking-high, 10 findings

Act On

Bearer JWT logged at verbose (and headers logger now prints it) — A/B/C/D. updateUserJwt still logs the full token (// TODO: omit… before shipping). Separately, OSRequestAuth.setBearer populates Authorization into additionalHeaders, and OneSignalClient already logs that dictionary verbatim — pre-IV that path was inert; it is now a credential sink whenever verbose is on. Redact both before any IV GA.
Parked Create User can complete after a user switch and report the wrong current user — A/D. _executePendingRequests steps over awaitsToken so another user’s work can run first; when the parked create later succeeds, parseFetchUserResponse → identityModel.hydrate → OSUserStateSnapshot.fireUserStateChanged is not gated by currentUser(matching:), while later hydration in the same function is. Add a regression test for login(A) (no token) → login(B) → updateUserJwt(A).
App-id change clears IV disk keys / retries without resetting in-memory gate — C (also flagged on #1707). handleAppIdChange removes OSUD_USE_IDENTITY_VERIFICATION / feature flags but leaves OSUserJwtConfig / OSFeatureManager memory; refreshIfUnknown / refreshIfEmpty then no-op. Stale scheduleDownloadIOSParamsRetryWithAppId: can still hydrate the previous app’s jwt_required into the shared singleton.
JWT persisted into shared UserDefaults archives — A/B/C/D. Encoding jwtBearerToken on OSIdentityModel (and thus model/request caches) expands exposure vs the prior in-memory-only + “make this token secure” TODO (removed). Persist via Keychain (or drop cross-launch persistence and re-ask), and stop embedding the secret in every archived request identity copy.
IAM deferred-fetch slot can be wiped by onUserWillChange with a nil subscription id — D. Single deferredFetchSubscriptionId; login while Create User is parked (no push sub id yet) overwrites a prior 401 park with nil, and retryDeferredFetch then no-ops for the rest of the session.

Consider

Feature-flag layer has no production writer — A/B/C/D. setEnabledFeatureKeys is test-only; newCodePathsRun collapses to ivBehaviorActive. Land remote-params delivery with the flag, or delete the dead branch until then.
Deltas stamped from live identityModel.externalId during clearUserData — D. Fetch clears aliases before hydrate; tags/aliases in that window stamp nil and are purged under IV.
Cached .off + async params hydrate — B. Unsigned mutations can 401 and be dropped before the session learns IV turned on.
currentUser(matching:) then mutate/logout is not atomic — B. Check-then-act across executors can still clear/logout the wrong user under a tight switch.
JWT-invalidated observer lazy init race — D. Lost listener + askedForToken can strand asks for the session.
RYW subscription bar uses sticky prior tokens / shared condition clear-on-timeout — B/A/D. Later subscription writes can look “ready” too early; timeout can lower the bar for a concurrent waiter.
IAM 401 never invalidates JWT — A. Documented ambiguity vs mid-session expiry with no user traffic; consider bounded invalidate after repeated signed 401s.
Logout unsubscribe vs immediate re-login create ordering — A. Concurrent executors can leave the device unsubscribed server-side.
Create/Fetch ownerExternalId still live-read — C. Dual ownership convention vs stamped requests; weak under clearData.

Noted

Pre-ownership / pre-addsNewRecords cache decode → purge or skip cool-down on upgrade (release note / decode default).
OSRequestGetInAppMessages encode-failure returns path-less request.
OneSignalUserManagerImpl.swift crossed ~1k lines despite +Jwt extraction.
Listener replay after remove / possible duplicate ask — B.

Dismissed

Rewrites that only prefer a different structure without a broken path (general “code judo” without concrete failure).
Re-litigating three-state jwt_required / hold-until-known design (intent of the stack).
Treating umbrella-vs-stacked-PR process as a code defect.

Agreement Map

Strong consensus on credential handling (verbose token log + UserDefaults persistence) and on dead OSFeatureManager producer. Two independent models (A/D) reconstructed the same parked-create → wrong OSUserStateObserver path after step-over. C resurfaced the #1707 app-id / stale-retry gate bugs still present in the tip. B uniquely stressed rollout-transition silent drops and RYW generation mismatch; D uniquely stressed IAM defer-nil cancel and observer lazy-init. Net: IV ownership/gating story is real, but shipping blockers are credential leakage/persistence, wrong-user observer after park-step-over, and app-id gate memory/retry consistency.

Inline comments below map to Act On / high-signal Consider items.

_{Sent by Cursor Automation: Untitled}

cursor · 2026-08-12T01:58:19Z

+            return
+        }
+        // TODO: omit the token from this log before shipping — keep for testing.
+        OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignal.updateUserJwt called for externalId: \(externalId) with token: \(token)")


Act On — credential in logs (consensus A/B/C/D)

This still logs the full bearer token at verbose, with an explicit pre-ship TODO. Separately, OSRequestAuth.setBearer puts Authorization: Bearer … into additionalHeaders, and OneSignalClient logs that dictionary verbatim — so every signed request becomes a second leak once verbose is on.

Omit the token here (externalId / presence only) and redact Authorization in the client logger before IV GA.

cursor · 2026-08-12T01:58:19Z

+        lock.withLock {
            super.encode(with: coder)
            coder.encode(aliases, forKey: "aliases")
+            coder.encode(jwtBearerTokenLocked, forKey: OS_JWT_BEARER_TOKEN)


Act On — JWT archived to shared UserDefaults (consensus A/B/C/D)

Encoding jwtBearerTokenLocked (and set(property:) → model-store save) persists the bearer into App Group UserDefaults / request archives. Pre-stack this was in-memory with a “make this token secure” TODO; the TODO was removed while exposure grew.

Prefer Keychain (external-id keyed) or session-only memory + re-ask; do not keep N copies of the secret in model/request plists.

cursor · 2026-08-12T01:58:19Z

            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) {


Act On — step-over + ungated observer (A/D)

Stepping over awaitsToken is necessary so another login is not stranded, but it makes a previously FIFO-impossible path real: parked CreateUser(A) can complete after CreateUser(B) already hydrated the current user.

parseFetchUserResponse still calls identityModel.hydrate (→ OSUserStateSnapshot.fireUserStateChanged) before the currentUser(matching:) guard used for properties/subs. Result: app observer / persisted snapshot can flip back to A while current is B (or after logout).

Gate the observer fire on current-user, and add a lifecycle test for login(A, no token) → login(B) → updateUserJwt(A).

cursor · 2026-08-12T01:58:19Z


        internalAddAliases(remoteAliases)
-        fireUserStateChanged(newOnesignalId: newOnesignalId, newExternalId: newExternalId)
+        OSUserStateSnapshot.fireUserStateChanged(newOnesignalId: newOnesignalId, newExternalId: newExternalId)


Act On — companion to parked-create race

fireUserStateChanged runs unconditionally from hydration. Callers that hydrate non-current models (see OSUserExecutor step-over of token-parked creates) will notify the app of the wrong current user.

Move this fire to call sites that know the model is current, or gate on currentUser(matching:).

cursor · 2026-08-12T01:58:19Z

        [sharedUserDefaults removeValueForKey:OS_PUSH_SUBSCRIPTION_MODEL_STORE_KEY];

+        [sharedUserDefaults removeValueForKey:OSUD_USE_IDENTITY_VERIFICATION];
+        [sharedUserDefaults removeValueForKey:OSUD_SDK_FEATURE_FLAGS];


Act On — disk clear without in-memory reset (C; also #1707)

App-id change removes OSUD_USE_IDENTITY_VERIFICATION / OSUD_SDK_FEATURE_FLAGS but does not reset OSUserJwtConfig.shared / OSFeatureManager.shared. refreshIfUnknown / refreshIfEmpty no-op when memory is already set, so the previous app’s gate can keep driving IV behavior until (or instead of) the new params hydrate.

Reset requirement to .unknown and clear feature keys here (same moment as the disk clear).

cursor · 2026-08-12T01:58:19Z

+        if (_downloadedParameters || _didCallDownloadParameters)
+            return;
+
+        [self downloadIOSParamsWithAppId:appId];


Act On — stale appId on params retry (C; also #1707)

This block captures the failing appId. handleAppIdChange resets download flags but does not cancel the delayed retry, so it can still call downloadIOSParamsWithAppId: for the old app and hydrate jwt_required into the shared config used by the new app.

Compare against OneSignalIdentifiers.currentAppId (or a generation stamp) before retrying.

cursor · 2026-08-12T01:58:19Z

+    @synchronized (self) {
+        self.userGeneration += 1;
+    }
+    [self deferFetchWithSubscriptionId:OneSignalUserManagerImpl.sharedInstance.pushSubscriptionId];


Act On — deferred IAM fetch erased by nil (D)

One slot: deferFetchWithSubscriptionId: overwrites unconditionally. Under IV, login while Create User is parked often has no push subscription id yet, so this writes nil and drops a fetch parked by handleUnauthorizedFetch: / “no onesignal id”. retryDeferredFetch then no-ops for the rest of the session.

Ignore nil here, or separate “fetch owed” from “subscription id to use”.

cursor · 2026-08-12T01:58:19Z

            name: OS_UPDATE_PROPERTIES_DELTA,
-            identityModelId: userInstance.identityModel.modelId,
+            identityModelId: identityModel.modelId,
+            externalId: identityModel.externalId,


Consider — live externalId stamp during clear/hydrate window (D)

Deltas take identityModel.externalId from the live model. clearUserData blanks aliases before fetch hydrate; work in that window stamps nil and OSOperationRepo drops it under IV (silent loss for an identified user).

Preserve external_id across clearData, or hydrate aliases atomically so an identified model never reads anonymous.

cursor · 2026-08-12T01:58:19Z

+        }
+    }
+
+    public func setEnabledFeatureKeys(_ keys: [String]) {


Consider — no production writer (A/B/C/D)

setEnabledFeatureKeys is only reached from tests; OneSignal.m never hydrates flags. newCodePathsRun therefore equals ivBehaviorActive in every real app, and the flag-only branch in OSRequestAuth.authorization is dead.

Ship remote-params flag delivery with this machinery, or collapse to ivBehaviorActive until the producer exists.

Outer iOS-params retries now follow OSResponseStatusRetryable so classified 4xx are not re-armed for the session. Hydration handlers snapshot requirement under the same lock as registration. Co-authored-by: Cursor <cursoragent@cursor.com>

The surface an app talks to under Identity Verification: it hands the SDK a token for a user, and the SDK tells it when that token stopped being accepted. OSUserJwtRepo holds the token per external ID and remembers who has already been asked for one, so an app is asked once per user rather than once per rejected request. A listener registered after start or login still hears about an ask that already fired, since the alternative is an app that never learns it owes a token for the user it just logged in. Removes the beta JWT surface this replaces: requiresUserAuth, which nothing ever read, along with onJwtExpired and its handler typealiases. Anyone on the beta JWT API moves to addUserJwtInvalidatedListener and updateUserJwt. storeJwt lands here in the form the public API needs; the later PR that introduces the queues extends it to release the work held for want of a token. Co-authored-by: Cursor <cursoragent@cursor.com>

Use preventServerUpdate so the token never becomes an identity delta by accident of the alias cast failing. Co-authored-by: Cursor <cursoragent@cursor.com>

A Delta records a change to a model but not who it was for, so anything built from one had to ask who the current user is. Between the change and the flush the app may have logged in as somebody else, and the queued work was then sent for whoever happened to be current. OSDelta now carries the external ID of the user it was made for, as a required argument so no call site can leave it out. The model store listeners take the operation repo by injection rather than reaching for the singleton, which is also what lets the next PR give the repo an Identity Verification-aware instance. Nothing reads the new field yet; the request pipeline picks it up next. Co-authored-by: Cursor <cursoragent@cursor.com>

…tamping Refuse to stamp the current user when the changed model is no longer theirs so a concurrent login cannot attach the wrong external_id for PR6 auth. Properties require the current properties model; email/SMS add requires the model still be in the store. Remove and push updates keep stamping the current user. Co-authored-by: Cursor <cursoragent@cursor.com>

Everything that sends a user-scoped call now decides how to address and sign it in one place. OSRequestAuth answers, for a given user, which alias names them in the path and which token signs it, read together so the alias and the token can never come from different users. Under Identity Verification a user is addressed by external_id, which the app chooses, so those path segments are percent-encoded through OSUrlPath. Requests carry the identity model that owns them rather than reading whoever is current at send time. The operation repo holds queued work while the requirement is still unknown, since sending unsigned would be rejected and sending signed too early is not possible. Once the answer arrives the queue flushes. When Identity Verification is on, work belonging to no external ID is dropped rather than sent: an anonymous user is never created on the server, so that work has no user to belong to. Update Subscription is deliberately exempt from all of this. A push subscription belongs to the device, not the signed-in user, so it always goes out whether or not anyone is logged in and whether or not a token is valid; nothing about it is gated on auth. Logging out while Identity Verification is on internally disables the push subscription, since the replacement anonymous user is never created and the subscription would otherwise keep reporting under the logged-out user. Logging back in clears that, as does the requirement hydrating to off. Co-authored-by: Cursor <cursoragent@cursor.com>

Under TEST, OP_REPO_POST_CREATE_DELAY_SECONDS is 0, so canAccess released an ID the instant it was added and the Requests left the executor queues before removeOperationsWithoutExternalId could see them. MockNewRecordsState.holdWhilePresent keeps an ID inaccessible for as long as it is present. Purge tests opt in; every other consumer keeps the production timer behavior. Co-authored-by: Cursor <cursoragent@cursor.com>

No behavior change — move code into extensions / a top-level OSPushSubscriptionImpl so file_length and type_body_length stay under error thresholds. Co-authored-by: Cursor <cursoragent@cursor.com>

The in-app message fetch is user-scoped, so under Identity Verification it has to name the user by external_id and sign the call. It asks the user manager how to address the current user rather than assembling that itself, which keeps the alias and the token consistent with every other user-scoped call. The fetch deliberately does not invalidate a JWT. Getting this request right means getting both the push subscription ID and the user in the URL right, so a 401 here is at least as likely to mean the request was mismatched as it is to mean the token is bad. Treating it as proof would let a malformed fetch invalidate a token that works everywhere else. It handles the rejection and stops; the request pipeline remains the only thing that decides a token is no longer good. This follows Android. A fetch that cannot yet be addressed waits rather than going out unsigned, and is reattempted when the requirement hydrates or the app supplies a token. Co-authored-by: Cursor <cursoragent@cursor.com>

Exercises the Identity Verification surface end to end: log in with a token, watch a token be rejected and supply a replacement through the invalidated listener, and see how the SDK behaves while the requirement is still unknown. Co-authored-by: Cursor <cursoragent@cursor.com>

nan-li and others added 5 commits August 11, 2026 16:34

chore: normalize project.pbxproj ordering

bf7c24d

Canonical sort applied by the xcodeproj tool, so later commits in this stack show only their own file additions. No project changes. Co-authored-by: Cursor <cursoragent@cursor.com>

chore: tighten RYW comments to iOS comment style

c09d5a9

Cut hazard essays and cross-user narration down to short whys hitched to the action, and trim the resolveConditions doc to the contract. Co-authored-by: Cursor <cursoragent@cursor.com>

cursor Bot reviewed Aug 12, 2026

View reviewed changes

nan-li force-pushed the nan/jwt-pr8-demo-ui branch from c865aed to 57f1f57 Compare August 12, 2026 06:34

nan-li and others added 2 commits August 12, 2026 09:50

nan-li force-pushed the nan/jwt-pr8-demo-ui branch from 57f1f57 to 2f108c9 Compare August 12, 2026 16:57

nan-li and others added 2 commits August 12, 2026 10:16

fix: [PR4] mark JWT bearer updates as local-only

5aea671

Use preventServerUpdate so the token never becomes an identity delta by accident of the alias cast failing. Co-authored-by: Cursor <cursoragent@cursor.com>

nan-li force-pushed the nan/jwt-pr8-demo-ui branch from 2f108c9 to 97717b9 Compare August 12, 2026 17:35

nan-li and others added 6 commits August 12, 2026 14:14

chore: [PR6] split types to clear SwiftLint length errors

a642d1c

No behavior change — move code into extensions / a top-level OSPushSubscriptionImpl so file_length and type_body_length stay under error thresholds. Co-authored-by: Cursor <cursoragent@cursor.com>

nan-li force-pushed the nan/jwt-pr8-demo-ui branch from 97717b9 to 9ac26b6 Compare August 12, 2026 21:22

Navigation Menu

Uh oh!

Identity Verification (JWT) — full stack for adversarial review - #1708

Identity Verification (JWT) — full stack for adversarial review#1708
nan-li wants to merge 15 commits into
mainfrom
nan/jwt-pr8-demo-ui

nan-li commented Aug 12, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

nan-li commented Aug 12, 2026

Summary

What’s in this diff (stacked slices)

Intent of the feature

Test plan

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Multimodal adversarial review (interrogate)

Intent

Reviewers

Act On

Consider

Noted

Dismissed

Agreement Map

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant